tr: Fetch Wait channel before killTask in restart

[endocrimes]

Currently, if killTask results in the termination of a process before
calling WaitTask, Restart() will incorrectly return a TaskNotFound
error when using the raw_exec driver on Windows.

This fetches the WaitCh before killing the process to avoid this race
condition.

[schmichael]

Great catch. Should kill have similar treatment or doesn't it matter since kill always transitions to a terminal state?

[endocrimes]

I’m debugging a similar issue during executor shutdown right now which might end up leading to something similar, but not fully sure yet.

[notnoop]

Good catch! Is this something we can test? Feels like a subtle thing that we might miss with enough refactoring.

Also, I'm very confused how the ordering causes TaskNotFound error, mind if you elaborate on how it gets to that state? In raw_exec, Seeing handle.Wait() only returns TaskNotFound if task was removed from d.tasks [1], which seems to only happen in driver.DestroyTask()[2]. Was there another place where task was deleted/destroyed?

[1]

nomad/drivers/rawexec/driver.go

Lines 383 to 387 in 079cfb4

	func (d *Driver) WaitTask(ctx context.Context, taskID string) (<-chan *drivers.ExitResult, error) {
	handle, ok := d.tasks.Get(taskID)
	if !ok {
	return nil, drivers.ErrTaskNotFound
	}

[2]

nomad/drivers/rawexec/driver.go

Line 461 in 079cfb4

d.tasks.Delete(taskID)

[endocrimes]

@notnoop DestroyTask gets called as part of terminating a tasks Run loop here: https://github.com/hashicorp/nomad/blob/dani/win-killer/client/allocrunner/taskrunner/task_runner.go#L482

testing this reliably seems pretty hard right now, because it's very much integration test-y and would need a full driver matrix - might be dooable when we invest in more test infra soon though.

[notnoop]

Ah - makes sense - I missed that call - thanks for the clarification!

[github-actions]

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.