Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release execution slot while waiting for task #1025

Merged

Conversation

theunrepentantgeek
Copy link
Contributor

When the second execution of a Task needs to be skipped because it has already run (or is currently running), the code holds onto an execution slot while it waits for the first execution of the task to complete.

If the first execution of the task has already completed, this wait immediately completes, and the execution slot is also immediately freed.

If the first execution of the task is running its dependencies, it has temporarily released its execution slot to allow the dependencies to be run. Once those are complete, it attempts to reacquire an execution slot before running the commands for the task.

If there is only one execution slot, this gets blocked because the only slot is held by the second execution of the task.

Closes #715

Reproducing

Reproducing this deadlock wasn't too bad thanks to the sample Taskfile provided in #715.

$ for i in $(seq 1 1000); do echo $i ; go run ./cmd/task/ -v -C 1 --dir ./deadlock; done

Running against master, deadlock will usually happen within the first dozen or so invocations.
Running against this branch, all 1000 iterations run cleanly.

Detailed analysis.

Normal case

Here's the normal case, when the first execution of the task (prefixed by [EXEC]) completes before the second execution (prefixed by [SKIP]).

[EXEC] First execution of task A
[EXEC] Channel Executor.concurrencySemaphore contains 0 active execution slots
[EXEC] Executor.RunTask() is called for task A (task.go L#127)
[EXEC] An execution slot is acquired so the task can run (task.go L#136)
[EXEC] Channel Executor.concurrencySemaphore contains 1 active execution slot
[EXEC] Release of the execution slot is deferred until later (task.go L#137)
[EXEC] Executor.runDeps() is called  to run the dependencies of A (task.go L#218)
[EXEC] The execution slot is temporarily released to allow dependencies to run (task.go L#221)
[EXEC] Channel Executor.concurrencySemaphore contains 0 active execution slots
[EXEC] Reacquisition of the execution slot is deferred until later (task.go L#222)
[EXEC] Dependencies of A are run (task.go L#224)
[EXEC] Execution slot is reacquired (task.go L#222)
[EXEC] Channel Executor.concurrencySemaphore contains 1 active execution slot
[EXEC] Executor.startExecution() is called to run the commands for A (task.go L#334)
[EXEC] Commands for A are run 
[EXEC] Execution slot is released (task.go L#137)
[EXEC] Channel Executor.concurrencySemaphore contains 0 active execution slots
[EXEC] First execution of task A completes

[SKIP] Second execution of task A
[SKIP] Executor.RunTask() is called for task A (task.go L#127)
[SKIP] An execution slot is acquired so the task can run (task.go L#136)
[SKIP] Channel Executor.concurrencySemaphore contains 1 active execution slot
[SKIP] Release of the execution slot is deferred until later (task.go L#137)
[SKIP] Executor.runDeps() is called  to run the dependencies of A (task.go L#218)
[SKIP] The execution slot is temporarily released to allow dependencies to run (task.go L#221)
[SKIP] Channel Executor.concurrencySemaphore contains 0 active execution slots
[SKIP] Reacquisition of the execution slot is deferred until later (task.go L#222)
[SKIP] Dependencies of A are run (task.go L#224)
[SKIP] Execution slot is reacquired (task.go L#222)
[SKIP] Channel Executor.concurrencySemaphore contains 1 active execution slot
[SKIP] Executor.startExecution() is called to run the commands for A (task.go L#334)
[SKIP] The map Executor.executionHashes is checked, and we discover the task has already run (task.go L#346).
[SKIP] We wait on the completion of the task (no delay, because it's already finished) (task.go L#349)
[SKIP] Execution slot is released (task.go L#137)
[SKIP] Channel Executor.concurrencySemaphore contains 0 active execution slots
[SKIP] Second execution of task A completes

Deadlock case

Here's the deadlock case, where the second execution of task A commences while the first execution is handling dependencies.

[EXEC] First execution of task A
[EXEC] Channel Executor.concurrencySemaphore contains 0 active execution slots
[EXEC] Executor.RunTask() is called for task A (task.go L#127)
[EXEC] An execution slot is acquired so the task can run (task.go L#136)
[EXEC] Channel Executor.concurrencySemaphore contains 1 active execution slot
[EXEC] Release of the execution slot is deferred until later (task.go L#137)
[EXEC] Executor.runDeps() is called  to run the dependencies of A (task.go L#218)
[EXEC] The execution slot is temporarily released to allow dependencies to run (task.go L#221)
[EXEC] Channel Executor.concurrencySemaphore contains 0 active execution slots
[EXEC] Reacquisition of the execution slot is deferred until later (task.go L#222)
[EXEC] Dependencies of A are run (task.go L#224)

[SKIP] Second execution of task A
[SKIP] Executor.RunTask() is called for task A (task.go L#127)
[SKIP] A execution slot is acquired so the task can run (task.go L#136)
[SKIP] Channel Executor.concurrencySemaphore contains 1 active execution slot
[SKIP] Release of the execution slot is deferred until later (task.go L#137)
[SKIP] Executor.runDeps() is called  to run the dependencies of A (task.go L#218)
[SKIP] The execution slot is temporarily released to allow dependencies to run (task.go L#221)
[SKIP] Channel Executor.concurrencySemaphore contains 0 active execution slots
[SKIP] Reacquisition of the execution slot is deferred until later (task.go L#222)
[SKIP] Dependencies of A are run (task.go L#224)
[SKIP] Execution slot is reacquired (task.go L#222)
[SKIP] Channel Executor.concurrencySemaphore contains 1 active execution slot
[SKIP] Executor.startExecution() is called to run the commands for A (task.go L#334)
[SKIP] The map Executor.executionHashes is checked, and we discover the task has already run (task.go L#346).
[SKIP] We wait on the completion of the task (blocking because the original task is still running) (task.go L#349)

[EXEC] Reacquiring the execution slot is attempted but blocks because no execution slot is available (task.go L#222)

@andreynering
Copy link
Member

Thanks for your contribution @theunrepentantgeek!

@andreynering andreynering merged commit 52756ab into go-task:master Mar 2, 2023
andreynering added a commit that referenced this pull request Mar 2, 2023
@theunrepentantgeek theunrepentantgeek deleted the fix/skipping-task-deadlock branch March 2, 2023 03:24
@pd93 pd93 mentioned this pull request Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stuck in deadlock with up-to-date "run once" task
2 participants