Skip to content

Commit

Permalink
Replace synchronized in JavaSubprocessFactory with ReentranctLock.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 721740709
Change-Id: I91d33c8eddc094eb47068b63b25843951acc94f3
  • Loading branch information
coeuvre authored and copybara-github committed Jan 31, 2025
1 parent 73317d0 commit 47f3253
Showing 1 changed file with 7 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.lang.ProcessBuilder.Redirect;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.locks.ReentrantLock;

/** A subprocess factory that uses {@link java.lang.ProcessBuilder}. */
public class JavaSubprocessFactory implements SubprocessFactory {
Expand Down Expand Up @@ -114,6 +115,7 @@ public long getProcessId() {
}

public static final JavaSubprocessFactory INSTANCE = new JavaSubprocessFactory();
private final ReentrantLock lock = new ReentrantLock();

private JavaSubprocessFactory() {
// We are a singleton
Expand All @@ -136,8 +138,9 @@ private JavaSubprocessFactory() {
// I was able to reproduce this problem reliably by running significantly more threads than
// there are CPU cores on my workstation - the more threads the more likely it happens.
//
// As a workaround, we put a synchronized block around the fork.
private synchronized Process start(ProcessBuilder builder) throws IOException {
// As a workaround, we use a lock around the fork.
private Process start(ProcessBuilder builder) throws IOException {
lock.lock();
try {
return builder.start();
} catch (IOException e) {
Expand All @@ -150,6 +153,8 @@ private synchronized Process start(ProcessBuilder builder) throws IOException {
e);
}
throw e;
} finally {
lock.unlock();
}
}

Expand Down

0 comments on commit 47f3253

Please sign in to comment.