Skip to content

Commit

Permalink
Fix issue with enforced tasks and task graph exception
Browse files Browse the repository at this point in the history
Fixes issue #2407 where an exception thrown while walking a task
graph with a finalizer can cause the build to hang.
  • Loading branch information
ghale committed Jul 12, 2017
1 parent efc9360 commit 5df5c26
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 4 deletions.
Expand Up @@ -21,6 +21,7 @@ import org.gradle.integtests.fixtures.executer.GradleContextualExecuter
import org.gradle.test.fixtures.server.http.BlockingHttpServer
import org.junit.Rule
import spock.lang.IgnoreIf
import spock.lang.Timeout

@IgnoreIf({ GradleContextualExecuter.parallel })
// no point, always runs in parallel
Expand Down Expand Up @@ -369,4 +370,34 @@ class ParallelTaskExecutionIntegrationTest extends AbstractIntegrationSpec {
run ":cPing", ":aPing"
}
@Timeout(30)
def "handles an exception thrown while walking the task graph when a finalizer is present"() {
given:
withParallelThreads(2)
buildFile << """
class BrokenTask extends DefaultTask {
@OutputFiles
FileCollection getOutputFiles() {
throw new RuntimeException('BOOM!')
}

@TaskAction
void doSomething() {
println "Executing broken task..."
}
}

task brokenTask(type: BrokenTask)
aPing.finalizedBy brokenTask
"""
expect:
blockingServer.expectConcurrent(":aPing")
fails ":aPing"
and:
failure.assertHasDescription("BOOM!")
}
}
Expand Up @@ -550,7 +550,7 @@ public ResourceLockState.Disposition transform(ResourceLockState resourceLockSta
try {
selected.set(selectNextTask(workerLease));
} catch (Throwable t) {
abortAndFail(t);
abortAllAndFail(t);
workRemaining.set(false);
}

Expand Down Expand Up @@ -864,8 +864,8 @@ private void enforceWithDependencies(TaskInfo nodeInfo, Set<TaskInfo> enforcedTa
}
}

private void abortAndFail(Throwable t) {
abortExecution();
private void abortAllAndFail(Throwable t) {
abortExecution(true);
this.failures.add(t);
}

Expand All @@ -890,13 +890,23 @@ private void handleFailure(TaskInfo taskInfo) {
}

private boolean abortExecution() {
// Allow currently executing and enforced tasks to complete, but skip everything else.
return abortExecution(false);
}

private boolean abortExecution(boolean abortAll) {
boolean aborted = false;
for (TaskInfo taskInfo : executionPlan.values()) {
// Allow currently executing and enforced tasks to complete, but skip everything else.
if (taskInfo.isRequired()) {
taskInfo.skipExecution();
aborted = true;
}

// If abortAll is set, also stop enforced tasks.
if (abortAll && taskInfo.isReady()) {
taskInfo.abortExecution();
aborted = true;
}
}
return aborted;
}
Expand Down
Expand Up @@ -98,6 +98,11 @@ public void skipExecution() {
state = TaskExecutionState.SKIPPED;
}

public void abortExecution() {
assert isReady();
state = TaskExecutionState.SKIPPED;
}

public void require() {
state = TaskExecutionState.SHOULD_RUN;
}
Expand Down
Expand Up @@ -20,12 +20,14 @@ import com.google.common.collect.Queues
import org.gradle.api.Action
import org.gradle.api.DefaultTask
import org.gradle.api.Task
import org.gradle.api.file.FileCollection
import org.gradle.api.internal.project.ProjectInternal
import org.gradle.api.tasks.Destroys
import org.gradle.api.tasks.InputDirectory
import org.gradle.api.tasks.InputFile
import org.gradle.api.tasks.OutputDirectory
import org.gradle.api.tasks.OutputFile
import org.gradle.api.tasks.OutputFiles
import org.gradle.initialization.BuildCancellationToken
import org.gradle.internal.nativeintegration.filesystem.FileSystem
import org.gradle.internal.resources.DefaultResourceLockCoordinationService
Expand Down Expand Up @@ -663,6 +665,24 @@ class DefaultTaskExecutionPlanParallelTest extends ConcurrentSpec {
operation."${c.path}".start > operation."${b.path}".end
}

def "handles an exception while while walking the task graph when an enforced task is present"() {
given:
Task finalizer = root.task("finalizer", type: BrokenTask)
Task finalized = root.task("finalized")
finalized.finalizedBy finalizer

when:
addToGraphAndPopulate(finalized)
async {
startTaskWorkers(2)
releaseTasks(finalized)
}

then:
executionPlan.executionPlan[finalized].isSuccessful()
executionPlan.executionPlan[finalizer].state == TaskInfo.TaskExecutionState.SKIPPED
}

private void addToGraphAndPopulate(Task... tasks) {
executionPlan.addToTaskGraph(Arrays.asList(tasks))
executionPlan.determineExecutionPlan()
Expand Down Expand Up @@ -736,4 +756,11 @@ class DefaultTaskExecutionPlanParallelTest extends ConcurrentSpec {
@InputDirectory
File inputDirectory
}

static class BrokenTask extends DefaultTask {
@OutputFiles
FileCollection getOutputFiles() {
throw new Exception("BOOM!")
}
}
}

0 comments on commit 5df5c26

Please sign in to comment.