Skip to content

Commit

Permalink
task runner: fix goroutine leak in prestart hook
Browse files Browse the repository at this point in the history
The task runner prestart hooks take a `joincontext` so they have the
option to exit early if either of two contexts are canceled: from
killing the task or client shutdown. Batch jobs (and any failed job)
exit without being shutdown from the server, so neither of the joined
contexts ever gets canceled and we leak the `joincontext` (48 bytes)
and its internal goroutine. Cancel the `joincontext` after the
prestart call exits to fix the leak.
  • Loading branch information
tgross committed Dec 23, 2021
1 parent e32f024 commit 9cc522d
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
3 changes: 3 additions & 0 deletions .changelog/11741.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
client: Fixed a memory and goroutine leak for batch tasks and any task that exits without being shut down from the server
```
4 changes: 3 additions & 1 deletion client/allocrunner/taskrunner/task_runner_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,9 @@ func (tr *TaskRunner) prestart() error {
// Run the prestart hook
// use a joint context to allow any blocking pre-start hooks
// to be canceled by either killCtx or shutdownCtx
joinedCtx, _ := joincontext.Join(tr.killCtx, tr.shutdownCtx)
joinedCtx, joinedCancel := joincontext.Join(tr.killCtx, tr.shutdownCtx)
defer joinedCancel()

var resp interfaces.TaskPrestartResponse
if err := pre.Prestart(joinedCtx, &req, &resp); err != nil {
tr.emitHookError(err, name)
Expand Down

0 comments on commit 9cc522d

Please sign in to comment.