Skip to content

Commit

Permalink
fix(executor-shell): race blocking goroutine
Browse files Browse the repository at this point in the history
A goroutine writes to `waitCh` and a `select` block reads from the
channel. The `select` block is already reading from the context. It
might be the case that the `waitCh` write will block forever because
`ctx.Done` returns and no one is reading from `waitCh`.

This is discussed in detail in https://songlh.github.io/paper/gcatch.pdf
`1. Introduction:`

> A previously unknown concurrency bug in Docker is shown
> in Figure 1. Function Exec() creates a child goroutine at line 5
> to duplicate the content of a.Reader. After the duplication, the
> child goroutine sends err to the parent goroutine through channel
> outDone to notify the parent about completion and any possible error (line 7). Since outDone is an unbuffered channel (line 3), the child
> blocks at line 7 until the parent receives from outDone. Meanwhile,
> the parent blocks at the select at line 9 until it either receives err
> from the child (line 10) or receives a message from ctx.Done() (line
> 13), indicating the entire task can be halted. If the message from
> ctx.Done() arrives earlier, or if the two messages arrive concurrently and Go’s runtime non-deterministically chooses the second
> case to execute, the parent will return from function Exec(). No
> other goroutine can pull messages from outDone, leaving the child
> goroutine permanently blocked at line 7.
  • Loading branch information
steve-mt committed May 31, 2021
1 parent c715666 commit 2b2b3e0
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion executors/shell/shell.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ func (s *executor) Run(cmd common.ExecutorCommand) error {
}

// Wait for process to finish
waitCh := make(chan error)
waitCh := make(chan error, 1)
go func() {
waitErr := c.Wait()
var exitErr *exec.ExitError
Expand Down

0 comments on commit 2b2b3e0

Please sign in to comment.