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. Some tasks 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. This primarily impacts batch jobs and any task that fails
or completes early such as non-sidecar prestart lifecycle tasks.
Cancel the `joincontext` after the prestart call exits to fix the
leak.
  • Loading branch information
tgross committed Dec 23, 2021
1 parent e32f024 commit e1bbf30
Show file tree
Hide file tree
Showing 2 changed files with 7 additions and 2 deletions.
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
```
6 changes: 4 additions & 2 deletions client/allocrunner/taskrunner/task_runner_hooks.go
Original file line number Diff line number Diff line change
Expand Up @@ -235,9 +235,11 @@ func (tr *TaskRunner) prestart() error {
}

// Run the prestart hook
// use a joint context to allow any blocking pre-start hooks
// use a join 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 e1bbf30

Please sign in to comment.