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.
This commit is contained in:
Eduard Heimbuch
2022-10-27 10:31:26 +02:00
committed by GitHub
parent 54081ccdc6
commit df2a91fafe
2 changed files with 20 additions and 7 deletions

View File

@@ -0,0 +1,2 @@
- type: fixed
description: Concurrent access during Copy-on-write ([#2143](https://github.com/scm-manager/scm-manager/pull/2143))

View File

@@ -21,9 +21,10 @@
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
* SOFTWARE. * SOFTWARE.
*/ */
package sonia.scm.store; package sonia.scm.store;
import com.google.common.util.concurrent.Striped;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
@@ -31,26 +32,36 @@ import java.io.IOException;
import java.nio.file.Files; import java.nio.file.Files;
import java.nio.file.Path; import java.nio.file.Path;
import java.util.UUID; 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 * 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. * on errors such as full disk.
* * <p>
* javasecurity:S2083: SonarQube thinks that the path (targetFile) is generated from an http header (HttpUtil), but * 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. * 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 { public final class CopyOnWrite {
private static final Logger LOG = LoggerFactory.getLogger(CopyOnWrite.class); private static final Logger LOG = LoggerFactory.getLogger(CopyOnWrite.class);
private CopyOnWrite() {} private static final Striped<Lock> concurrencyLock = Striped.lock(10);
private CopyOnWrite() {
}
public static void withTemporaryFile(FileWriter writer, Path targetFile) { public static void withTemporaryFile(FileWriter writer, Path targetFile) {
validateInput(targetFile); validateInput(targetFile);
Path temporaryFile = createTemporaryFile(targetFile); Lock lock = concurrencyLock.get(targetFile.toString());
executeCallback(writer, targetFile, temporaryFile); try {
replaceOriginalFile(targetFile, temporaryFile); lock.lock();
Path temporaryFile = createTemporaryFile(targetFile);
executeCallback(writer, targetFile, temporaryFile);
replaceOriginalFile(targetFile, temporaryFile);
} finally {
lock.unlock();
}
} }
@SuppressWarnings("squid:S3725") // performance of Files#isDirectory @SuppressWarnings("squid:S3725") // performance of Files#isDirectory