Skip to content

Commit

Permalink
Merge pull request #7030 from mbien/fix-gradle-daemon-executor-concur…
Browse files Browse the repository at this point in the history
…rency

Fixed concurrency issues in GradleDaemonExecutor
  • Loading branch information
mbien authored Mar 3, 2024
2 parents 67cc930 + dcb2580 commit 736cf60
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,9 @@
import java.util.Collection;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import java.util.logging.Level;
import java.util.logging.Logger;
import javax.swing.AbstractAction;
Expand Down Expand Up @@ -439,41 +441,70 @@ private static String gradleExecutable() {
return Utilities.isWindows() ? "bin\\gradle.bat" : "bin/gradle"; //NOI18N
}

public final ExecutorTask createTask(ExecutorTask process) {
// TODO one of the two methods can likely go from the API
// setTask is called first and signals the runnable to start
// the started runnable requires (!) the gradleTask so it can't be created in createTask
@Override
public void setTask(ExecutorTask task) {
assert gradleTask == null;
gradleTask = new GradleTask(process);
gradleTask = new GradleTask(task);
super.setTask(task);
}

@Override
public final ExecutorTask createTask(ExecutorTask process) {
assert gradleTask != null;
assert task == process;
return gradleTask;
}

// task which can finish early, like a CompletableFuture
private static final class GradleTask extends ExecutorTask {
private final ExecutorTask delegate;
private Integer result;

private final ExecutorTask delegate;
private volatile Integer result;
// is 0 when wrapper or delegate finished
private final CountDownLatch doneSignal = new CountDownLatch(1);

GradleTask(ExecutorTask delegate) {
super(() -> {});
this.delegate = delegate;
this.delegate.addTaskListener(t -> doneSignal.countDown());
}

@Override
public void stop() {
this.delegate.stop();
delegate.stop();
}

@Override
public int result() {
if (result != null) {
return result;
}
return this.delegate.result();
waitFinished();
return result != null ? result : delegate.result();
}

@Override
public InputOutput getInputOutput() {
return this.delegate.getInputOutput();
return delegate.getInputOutput();
}

// FIXME isFinished() is final... this is still broken

@Override
public boolean waitFinished(long milliseconds) throws InterruptedException {
return doneSignal.await(milliseconds, TimeUnit.MILLISECONDS);
}

@Override
public void waitFinished() {
try {
doneSignal.await();
} catch (InterruptedException ex) {}
}

public void finish(int result) {
this.result = result;
doneSignal.countDown();
notifyFinished();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,13 +70,13 @@
private static final IOTable taskIOs = new IOTable(base, systemIO);

/* table of window:threadgrp */
private static WindowTable wtable = new WindowTable();
private static final WindowTable wtable = new WindowTable();

/** list of ExecutionListeners */
private HashSet<ExecutionListener> executionListeners = new HashSet<ExecutionListener>();
private final HashSet<ExecutionListener> executionListeners = new HashSet<>();

/** List of running executions */
private List<ExecutorTask> runningTasks = Collections.synchronizedList(new ArrayList<ExecutorTask>( 5 ));
private final List<ExecutorTask> runningTasks = Collections.synchronizedList(new ArrayList<>(5));

static {
systemIO.out = new OutputStreamWriter(System.out);
Expand Down Expand Up @@ -131,13 +131,14 @@ public String getRunningTaskName( ExecutorTask task ) {
* @param executor to start
* @param info about class to start
*/
@Override
public ExecutorTask execute(String name, Runnable run, final InputOutput inout) {
TaskThreadGroup g = new TaskThreadGroup(base, "exec_" + name + "_" + number); // NOI18N
g.setDaemon(true);
ExecutorTaskImpl task = new ExecutorTaskImpl();
synchronized (task.lock) {
try {
new RunClassThread(g, name, number++, inout, this, task, Lookup.getDefault(), run);
new RunClassThread(g, name, number++, inout, this, task, Lookup.getDefault(), run).start();
task.lock.wait();
} catch (InterruptedException e) { // #171795
inout.closeInputOutput();
Expand All @@ -157,6 +158,7 @@ public ExecutorTask execute(String name, Runnable run, final InputOutput inout)
*
* @return class path to libraries
*/
@Override
protected NbClassPath createLibraryPath() {
List<File> l = Main.getModuleSystem().getModuleJars();
return new NbClassPath (l.toArray (new File[0]));
Expand All @@ -181,6 +183,7 @@ public final void removeExecutionListener (ExecutionListener l) {
* @param io an InputOutput
* @return PermissionCollection for given CodeSource and InputOutput
*/
@Override
protected final PermissionCollection createPermissions(CodeSource cs, InputOutput io) {
PermissionCollection pc = Policy.getPolicy().getPermissions(cs);
ThreadGroup grp = Thread.currentThread().getThreadGroup();
Expand Down Expand Up @@ -252,6 +255,7 @@ static class SysOut extends OutputStream {
this.std = std;
}

@Override
public void write(int b) throws IOException {
if (std) {
getTaskIOs().getOut().write(b);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,6 @@ public RunClassThread(TaskThreadGroup base,
this.originalLookup = originalLookup;
// #33789 - this thread must not be daemon otherwise it is immediately destroyed
setDaemon(false);
this.start();
}

/** runs the thread
Expand Down Expand Up @@ -166,6 +165,7 @@ private void doRun() {
}
} // run method

@Override
public InputOutput getInputOutput() {
return io;
}
Expand Down

0 comments on commit 736cf60

Please sign in to comment.