made LockFile reentrant (multiple locks on the same file can be acquired in the same JVM instance)

added argument file to JavaProcessBuilder

gradle.properties

@@ -3,6 +3,6 @@ org.gradle.parallel=true
 org.gradle.caching=true
 
 gitea.maven.url = https://gitea.woggioni.net/api/packages/woggioni/maven
-jwo.version = 2025.01.22
+jwo.version = 2025.01.23
 lys.version = 2025.01.17
 guice.version = 5.0.1

jwo-test/build.gradle

@@ -1,11 +1,15 @@
 plugins {
     id 'java-library'
     alias(catalog.plugins.lombok)
+    alias(catalog.plugins.envelope)
 }
 
 import org.gradle.api.attributes.LibraryElements
 import static org.gradle.api.attributes.LibraryElements.LIBRARY_ELEMENTS_ATTRIBUTE
 import static org.gradle.api.attributes.LibraryElements.JAR
+import net.woggioni.gradle.envelope.EnvelopeJarTask
+import net.woggioni.gradle.envelope.EnvelopePlugin
+
 
 configurations {
     testImplementation {
@@ -25,7 +29,23 @@ java {
     modularity.inferModulePath = true
 }
 
+Provider<EnvelopeJarTask> envelopeJarTaskProvider = tasks.named(EnvelopePlugin.ENVELOPE_JAR_TASK_NAME, EnvelopeJarTask.class) {
+    mainModule = "net.woggioni.jwo.lockfile.test"
+    mainClass = "net.woggioni.jwo.lockfile.test.LockFileTestMain"
+}
+
 dependencies {
+    implementation rootProject
+
+    testImplementation catalog.slf4j.api
     testImplementation project(':jwo-test-module')
     testImplementation project(':')
+
+    testRuntimeOnly catalog.slf4j.simple
+}
+
+test {
+    dependsOn(envelopeJarTaskProvider)
+    systemProperty('lockFileTest.executable.jar', envelopeJarTaskProvider.flatMap {it.archiveFile}.get().getAsFile())
+    systemProperty('org.slf4j.simpleLogger.showDateTime', 'true')
 }

jwo-test/src/main/java/module-info.java

@@ -0,0 +1,6 @@
+module net.woggioni.jwo.lockfile.test {
+    requires net.woggioni.jwo;
+    requires static lombok;
+
+    exports net.woggioni.jwo.lockfile.test;
+}

jwo-test/src/main/java/net/woggioni/jwo/lockfile/test/LockFileTestMain.java

@@ -0,0 +1,42 @@
+package net.woggioni.jwo.lockfile.test;
+
+import lombok.SneakyThrows;
+import net.woggioni.jwo.LockFile;
+import net.woggioni.jwo.Run;
+
+import java.nio.file.Path;
+import java.nio.file.Paths;
+
+
+public class LockFileTestMain {
+
+    @SneakyThrows
+    private static void run(
+        Path lockfilePath,
+        boolean shared,
+        boolean keep
+    ) {
+        Thread t = new Thread((Run)() -> {
+            try (AutoCloseable lockfile = LockFile.acquire(lockfilePath, shared)) {
+                while (keep) {
+                    Thread.sleep(1000);
+                }
+            }
+        });
+        t.start();
+        try (AutoCloseable lockfile = LockFile.acquire(lockfilePath, shared)) {
+            while (keep) {
+                Thread.sleep(1000);
+            }
+        }
+        t.join();
+    }
+
+    @SneakyThrows
+    public static void main(String[] args) {
+        Path lockfilePath = Paths.get(args[0]);
+        boolean shared = Boolean.parseBoolean(args[1]);
+        boolean keep = Boolean.parseBoolean(args[2]);
+        run(lockfilePath, shared, keep);
+    }
+}

jwo-test/src/test/java/module-info.java

@@ -1,4 +1,5 @@
 open module net.woggioni.jwo.unit.test {
+    requires org.slf4j;
     requires net.woggioni.jwo;
     requires net.woggioni.jwo.test.module;
     requires org.junit.jupiter.api;

jwo-test/src/test/java/net/woggioni/jwo/test/LockFileTest.java

@@ -0,0 +1,147 @@
+package net.woggioni.jwo.test;
+
+
+import lombok.RequiredArgsConstructor;
+import lombok.SneakyThrows;
+import net.woggioni.jwo.JavaProcessBuilder;
+import net.woggioni.jwo.LockFile;
+import net.woggioni.jwo.Run;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.OutputStreamWriter;
+import java.io.Writer;
+import java.nio.channels.Channels;
+import java.nio.channels.FileChannel;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.nio.file.StandardOpenOption;
+import java.util.Arrays;
+import java.util.EnumSet;
+import java.util.List;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
+
+public class LockFileTest {
+    private static Logger log = LoggerFactory.getLogger(LockFileTest.class);
+
+    @TempDir
+    public Path testDir;
+
+    private Path executablePath = Paths.get(System.getProperty("lockFileTest.executable.jar"));
+
+    @RequiredArgsConstructor
+    private static class LockFileTestMainArgs {
+        final Path lockFilePath;
+        final boolean shared;
+        final boolean keep;
+
+        public List<String> getArgs() {
+            return Arrays.asList(lockFilePath.toString(), Boolean.toString(shared), Boolean.toString(keep));
+        }
+    }
+
+    @SneakyThrows
+    private static void kill(Process p) {
+        if (p != null && p.isAlive()) p.destroyForcibly().waitFor();
+    }
+
+    @Test
+    @SneakyThrows
+    public void testExclusiveLockHeldOnFile() {
+        Path lockFilePath = Files.createFile(testDir.resolve("file.lock"));
+        // try to acquire an exclusive lock and check that the process returns immediately
+        JavaProcessBuilder javaProcessBuilder = new JavaProcessBuilder();
+        javaProcessBuilder.setExecutableJar(executablePath);
+        javaProcessBuilder.setCliArgs(new LockFileTestMainArgs(lockFilePath, false, false).getArgs());
+        Process process = javaProcessBuilder.build()
+                .inheritIO()
+                .start();
+        Assertions.assertTrue(process.waitFor(1, TimeUnit.SECONDS));
+        Assertions.assertEquals(0, process.exitValue());
+
+        Process sharedLockProcess = null;
+        Process anotherSharedLockProcess = null;
+        Process exclusiveLockProcess = null;
+        try {
+            // try to acquire and keep a shared lock on the file and check that the process does not exit
+            javaProcessBuilder.setCliArgs(new LockFileTestMainArgs(lockFilePath, true, true).getArgs());
+            sharedLockProcess = javaProcessBuilder.build()
+                    .inheritIO()
+                    .start();
+            Assertions.assertFalse(sharedLockProcess.waitFor(1000, TimeUnit.MILLISECONDS));
+
+            // try to acquire another shared lock on the file and check that the process is able to terminate
+            javaProcessBuilder.setCliArgs(new LockFileTestMainArgs(lockFilePath, true, false).getArgs());
+            anotherSharedLockProcess = javaProcessBuilder.build()
+                    .inheritIO()
+                    .start();
+            Assertions.assertTrue(anotherSharedLockProcess.waitFor(1, TimeUnit.SECONDS));
+
+            // try to acquire an exclusive lock on the file and check that process hangs
+            javaProcessBuilder.setCliArgs(new LockFileTestMainArgs(lockFilePath, false, false).getArgs());
+            exclusiveLockProcess = javaProcessBuilder.build()
+                    .inheritIO()
+                    .start();
+            Assertions.assertFalse(exclusiveLockProcess.waitFor(1, TimeUnit.SECONDS));
+            // kill the process holding the shared lock and check that the process holding the exclusive lock terminates
+            sharedLockProcess.destroyForcibly().waitFor();
+            Assertions.assertTrue(exclusiveLockProcess.waitFor(1, TimeUnit.SECONDS));
+            Assertions.assertEquals(0, exclusiveLockProcess.exitValue());
+        } finally {
+            kill(sharedLockProcess);
+            kill(anotherSharedLockProcess);
+            kill(exclusiveLockProcess);
+        }
+    }
+
+    @Test
+    @SneakyThrows
+    public void sameProcessTest(@TempDir Path testDir) {
+        ExecutorService executor = Executors.newThreadPerTaskExecutor(Thread::new);
+        Path lockfile = testDir.resolve("file.lock");
+        AtomicInteger readerRunning = new AtomicInteger(0);
+        AtomicBoolean writerRunning = new AtomicBoolean(false);
+        Run writerRunnable = () -> {
+            try(LockFile lock = LockFile.acquire(lockfile, false)) {
+                log.info("Writer start!!!!");
+                writerRunning.set(true);
+                FileChannel fileChannel = FileChannel.open(lockfile, EnumSet.of(StandardOpenOption.CREATE, StandardOpenOption.READ, StandardOpenOption.WRITE));
+                Writer writer = new OutputStreamWriter(Channels.newOutputStream(fileChannel));
+                writer.write("asdffdgkhjdhigsdfhuifg");
+                Thread.sleep(100);
+                log.info("Writer end!!!!");
+                writerRunning.set(false);
+                Assertions.assertEquals(0, readerRunning.get());
+            }
+        };
+        Run readerRunnable = () -> {
+            try(AutoCloseable lock = LockFile.acquire(lockfile, true)) {
+                readerRunning.incrementAndGet();
+                log.info("reader start");
+                Thread.sleep(100);
+                log.info("reader end");
+                readerRunning.decrementAndGet();
+                Assertions.assertEquals(false, writerRunning.get());
+            }
+        };
+        CompletableFuture<?> reader1 = CompletableFuture.runAsync(readerRunnable, executor);
+        CompletableFuture<?> reader2 = CompletableFuture.runAsync(readerRunnable, executor);
+        CompletableFuture<?> writer = CompletableFuture.runAsync(writerRunnable, executor);
+        try {
+            CompletableFuture.allOf(reader1, reader2, writer).get();
+        } catch (ExecutionException ee) {
+            throw ee.getCause();
+        }
+        log.info("FINISHED");
+    }
+}

src/main/java/net/woggioni/jwo/JavaProcessBuilder.java

@@ -1,60 +1,167 @@
 package net.woggioni.jwo;
 
-import lombok.RequiredArgsConstructor;
+import lombok.AllArgsConstructor;
+import lombok.Getter;
+import lombok.Setter;
 import lombok.SneakyThrows;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
+import java.io.Writer;
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Arrays;
+import java.text.CharacterIterator;
+import java.text.StringCharacterIterator;
+import java.util.ArrayList;
 import java.util.List;
-import java.util.Optional;
+import java.util.Map;
 import java.util.Properties;
 import java.util.stream.Collectors;
-import java.util.stream.Stream;
 
-@RequiredArgsConstructor
+@Getter
+@Setter
 public class JavaProcessBuilder {
 
-    private final Class<?> mainClass;
+    private static final Logger log = LoggerFactory.getLogger(JavaProcessBuilder.class);
+
+    private static final String PROCESS_BUILDER_PREFIX = "javaProcessBuilder";
+
+    /**
+     * Maximum number of characters to be used to create a command line
+     * (beyond which a Java argument file will be created instead), the actual limit is OS-specific,
+     * the current value is extremely conservative so that it is safe to use on most operating systems
+     */
+    private static final int COMMAND_LINE_MAX_SIZE = 1024;
+
+    private static final String PATH_SEPARATOR = System.getProperty("path.separator");
+
+    @AllArgsConstructor
+    public static class JavaAgent {
+        Path jar;
+        String args;
+    }
+
+    private String mainClassName;
+
+    private Path executableJar;
+
+    private final String javaHome = System.getProperty("java.home");
+
+    private List<String> jvmArgs = new ArrayList<>();
+
+    private List<String> classpath = new ArrayList<>();
 
-    private String javaHome = System.getProperty("java.home");
-    private String classPath = System.getProperty("java.class.path");
     private Properties properties = new Properties();
-    private String[] cliArgs = null;
 
-    public JavaProcessBuilder javaHome(String javaHome) {
-        this.javaHome = javaHome;
-        return this;
+    private List<String> cliArgs = new ArrayList<>();
+
+    private List<JavaAgent> javaAgents = new ArrayList<>();
+
+    /**
+     * Generate the argument file string according to the grammar specified
+     * <a href="https://docs.oracle.com/en/java/javase/14/docs/specs/man/java.html#java-command-line-argument-files">here</a>
+     * @param strings list of command line arguments to be passed to the spawned JVM
+     * @return the Java argument file content as a string
+     */
+    static String generateArgumentFileString(List<String> strings) {
+        StringBuilder sb = new StringBuilder();
+        int i = 0;
+        while(i < strings.size()) {
+            CharacterIterator it = new StringCharacterIterator(strings.get(i));
+            sb.append('"');
+            for (char c = it.first(); c != CharacterIterator.DONE; c = it.next()) {
+                switch (c) {
+                    case '"':
+                        sb.append("\\\"");
+                        break;
+                    case '\r':
+                        sb.append("\\r");
+                        break;
+                    case '\n':
+                        sb.append("\\n");
+                        break;
+                    case '\t':
+                        sb.append("\\t");
+                        break;
+                    case '\\':
+                        sb.append("\\\\");
+                        break;
+                    default:
+                        sb.append(c);
+                        break;
                 }
-
-    public JavaProcessBuilder classPath(String classPath) {
-        this.classPath = classPath;
-        return this;
             }
-
-    public JavaProcessBuilder properties(Properties properties) {
-        this.properties = properties;
-        return this;
+            sb.append('"');
+            if(++i < strings.size()) {
+                sb.append(' ');
             }
-
-    public JavaProcessBuilder cliArgs(String ...cliArgs) {
-        this.cliArgs = cliArgs;
-        return this;
+        }
+        return sb.toString();
     }
 
     @SneakyThrows
-    public ProcessBuilder exec() {
+    public ProcessBuilder build() {
+        ArrayList<String> cmd = new ArrayList<>();
         Path javaBin = Paths.get(javaHome, "bin", "java");
-        Stream<String> propertyStream = Optional.ofNullable(properties)
-            .map(p -> p.entrySet().stream())
-            .orElse(Stream.empty())
-            .map(entry -> String.format("-D%s=%s", entry.getKey(), entry.getValue()));
-        List<String> cmd = JWO.streamCat(
-            Stream.of(javaBin.toString(), "-cp", classPath),
-            propertyStream,
-            Stream.of(mainClass.getCanonicalName()),
-            Optional.ofNullable(cliArgs).map(Arrays::stream).orElse(Stream.empty()))
-            .collect(Collectors.toList());
+        cmd.add(javaBin.toString());
+        cmd.addAll(jvmArgs);
+        if(!classpath.isEmpty()) {
+            cmd.add("-cp");
+            cmd.add(String.join(PATH_SEPARATOR, classpath));
+        }
+        for(Map.Entry<Object, Object> entry : properties.entrySet()) {
+            cmd.add(String.format("-D%s=%s", entry.getKey(), entry.getValue()));
+        }
+        for(JavaAgent javaAgent : javaAgents) {
+            StringBuilder sb = new StringBuilder("-javaagent:").append(javaAgent.jar.toString());
+            String agentArguments = javaAgent.args;
+            if(agentArguments != null) {
+                sb.append('=');
+                sb.append(agentArguments);
+            }
+            cmd.add(sb.toString());
+        }
+        if(executableJar != null) {
+            cmd.add("-jar");
+            cmd.add(executableJar.toString());
+        } else if(mainClassName != null) {
+            cmd.add(mainClassName);
+        } else {
+            throw new IllegalArgumentException(
+                    "Either a main class or the path to an executable jar file have to be specified");
+        }
+        cmd.addAll(cliArgs);
+
+        int cmdLength = 0;
+        for(String part : cmd) {
+            cmdLength += part.length();
+        }
+        //Add space between arguments
+        cmdLength += cmd.size() - 1;
+
+        if(log.isDebugEnabled()) {
+            log.debug("Spawning new process with command line: [{}]",
+                    cmd.stream().map(s -> "\"" + s + "\"").collect(Collectors.joining(", ")));
+        }
+        int jvmVersion = Integer.parseInt(System.getProperty("java.vm.specification.version"));
+        if(jvmVersion < 9 /* Java versions 8 and earlier do not support argument files */ || cmdLength < COMMAND_LINE_MAX_SIZE || cmd.size() == 1) {
             return new ProcessBuilder(cmd);
+        } else {
+            Path argumentFile = Files.createTempFile(PROCESS_BUILDER_PREFIX, ".arg");
+            Runtime.getRuntime().addShutdownHook(new Thread(() -> {
+                try {
+                    Files.delete(argumentFile);
+                } catch (IOException ioe) {
+                    ioe.printStackTrace();
+                }
+            }));
+            log.trace("Using Java argument file '{}'", argumentFile);
+            try(Writer writer = Files.newBufferedWriter(argumentFile)) {
+                writer.write(generateArgumentFileString(cmd.subList(1, cmd.size())));
+            }
+            return new ProcessBuilder(cmd.get(0), "@" + argumentFile);
+        }
     }
 }

src/main/java/net/woggioni/jwo/LockFile.java

@@ -1,40 +1,126 @@
 package net.woggioni.jwo;
 
-import java.io.Closeable;
+import lombok.Getter;
+import lombok.RequiredArgsConstructor;
+import lombok.Setter;
+
 import java.io.IOException;
 import java.nio.channels.FileChannel;
 import java.nio.channels.FileLock;
 import java.nio.file.Files;
 import java.nio.file.Path;
 import java.nio.file.StandardOpenOption;
+import java.util.Collections;
 import java.util.EnumSet;
+import java.util.HashMap;
+import java.util.Map;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReadWriteLock;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
-public class LockFile implements Closeable {
 
-    private final FileLock lock;
+@RequiredArgsConstructor
+public abstract class LockFile implements AutoCloseable {
+
+    @Getter
+    private static class LockFileMapValue {
+        private final ReadWriteLock threadLock;
+        private final AtomicInteger readerCount;
+
+        @Setter
+        private FileLock fileLock;
+
+        public LockFileMapValue(Path path) {
+            threadLock = new ReentrantReadWriteLock();
+            readerCount = new AtomicInteger(0);
+            fileLock = null;
+        }
+    }
+
+    private static Map<Path, LockFileMapValue> map = Collections.synchronizedMap(new HashMap<>());
 
     private static FileChannel openFileChannel(Path path) throws IOException {
         Files.createDirectories(path.getParent());
         return FileChannel.open(path, EnumSet.of(StandardOpenOption.CREATE, StandardOpenOption.READ, StandardOpenOption.WRITE));
     }
 
-    public static LockFile acquire(Path path, boolean shared) throws IOException {
+    private static LockFile acquireInternal(Path path, boolean shared, Boolean blocking) throws IOException {
+        LockFileMapValue lockFileMapValue = map.computeIfAbsent(path, LockFileMapValue::new);
+        if(shared) {
+            Lock lock = lockFileMapValue.getThreadLock().readLock();
+            if(blocking) {
+                lock.lock();
+            } else {
+                if(!lock.tryLock()) {
+                    return null;
+                }
+            }
+
+            int readers = lockFileMapValue.getReaderCount().incrementAndGet();
+            if(readers == 1) {
                 FileChannel channel = openFileChannel(path);
-        return new LockFile(channel.lock(0L, Long.MAX_VALUE, shared));
+                FileLock fileLock;
+                if(blocking) {
+                    fileLock = channel.lock(0L, Long.MAX_VALUE, true);
+                } else {
+                    fileLock = channel.tryLock(0L, Long.MAX_VALUE, true);
+                    if(fileLock == null) return null;
+                }
+                lockFileMapValue.setFileLock(fileLock);
+            }
+            return new LockFile() {
+                @Override
+                public void close() throws IOException {
+                    int remainingReaders = lockFileMapValue.getReaderCount().decrementAndGet();
+                    if(remainingReaders == 0) {
+                        FileLock fileLock = lockFileMapValue.getFileLock();
+                        fileLock.release();
+                        fileLock.channel().close();
+                        lockFileMapValue.setFileLock(null);
+                    }
+                    lock.unlock();
+                }
+            };
+        } else {
+            Lock lock = lockFileMapValue.getThreadLock().writeLock();
+            if(blocking) {
+                lock.lock();
+            } else {
+                if(!lock.tryLock()) {
+                    return null;
+                }
+            }
+            FileLock fileLock;
+            FileChannel channel = openFileChannel(path);
+            if(blocking) {
+                fileLock = channel.lock(0L, Long.MAX_VALUE, false);
+            } else {
+                fileLock = channel.tryLock(0L, Long.MAX_VALUE, false);
+                if(fileLock == null) {
+                    lock.unlock();
+                    return null;
+                }
+            }
+            lockFileMapValue.setFileLock(fileLock);
+            final FileLock fl = fileLock;
+            return new LockFile() {
+                @Override
+                public void close() throws IOException {
+                    fl.release();
+                    fl.channel().close();
+                    lockFileMapValue.setFileLock(null);
+                    lock.unlock();
+                }
+            };
+        }
     }
 
     public static LockFile tryAcquire(Path path, boolean shared) throws IOException {
-        FileChannel channel = openFileChannel(path);
-        FileLock lock = channel.tryLock(0L, Long.MAX_VALUE, shared);
-        return (lock != null) ? new LockFile(lock) : null;
+        return acquireInternal(path, shared, false);
     }
 
-    private LockFile(FileLock lock) {
-        this.lock = lock;
-    }
-
-    @Override
-    public void close() throws IOException {
-        lock.channel().close();
+    public static LockFile acquire(Path path, boolean shared) throws IOException {
+        return acquireInternal(path, shared, true);
     }
 }