From df2a91fafe3def9978fcee52680572ce99c6b6d2 Mon Sep 17 00:00:00 2001 From: Eduard Heimbuch Date: Thu, 27 Oct 2022 10:31:26 +0200 Subject: [PATCH] Prevent concurrent access during Copy-on-Write (#2143) Prevent concurrent access on files during Copy-on-Write which could led to inconsistent or corrupt files. --- gradle/changelog/copy_on_write.yaml | 2 ++ .../java/sonia/scm/store/CopyOnWrite.java | 25 +++++++++++++------ 2 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 gradle/changelog/copy_on_write.yaml diff --git a/gradle/changelog/copy_on_write.yaml b/gradle/changelog/copy_on_write.yaml new file mode 100644 index 0000000000..936c7fbbf4 --- /dev/null +++ b/gradle/changelog/copy_on_write.yaml @@ -0,0 +1,2 @@ +- type: fixed + description: Concurrent access during Copy-on-write ([#2143](https://github.com/scm-manager/scm-manager/pull/2143)) diff --git a/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java b/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java index 576b89562f..03807e3fb0 100644 --- a/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java +++ b/scm-dao-xml/src/main/java/sonia/scm/store/CopyOnWrite.java @@ -21,9 +21,10 @@ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * SOFTWARE. */ - + package sonia.scm.store; +import com.google.common.util.concurrent.Striped; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -31,26 +32,36 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.util.UUID; +import java.util.concurrent.locks.Lock; /** * CopyOnWrite creates a copy of the target file, before it is modified. This should prevent empty or incomplete files * on errors such as full disk. - * + *

* javasecurity:S2083: SonarQube thinks that the path (targetFile) is generated from an http header (HttpUtil), but * this is not true. It looks like a false-positive, so we suppress the warning for now. */ -@SuppressWarnings("javasecurity:S2083") +@SuppressWarnings({"javasecurity:S2083", "UnstableApiUsage"}) public final class CopyOnWrite { private static final Logger LOG = LoggerFactory.getLogger(CopyOnWrite.class); - private CopyOnWrite() {} + private static final Striped concurrencyLock = Striped.lock(10); + + private CopyOnWrite() { + } public static void withTemporaryFile(FileWriter writer, Path targetFile) { validateInput(targetFile); - Path temporaryFile = createTemporaryFile(targetFile); - executeCallback(writer, targetFile, temporaryFile); - replaceOriginalFile(targetFile, temporaryFile); + Lock lock = concurrencyLock.get(targetFile.toString()); + try { + lock.lock(); + Path temporaryFile = createTemporaryFile(targetFile); + executeCallback(writer, targetFile, temporaryFile); + replaceOriginalFile(targetFile, temporaryFile); + } finally { + lock.unlock(); + } } @SuppressWarnings("squid:S3725") // performance of Files#isDirectory