Seems like this could block if the for loop in the goroutine in handleInitExit has already received an event but hasn't run the defer.

Let me try to think through this:

• Line 284 is s.runningExecs[container]++, protected by s.lifecycleMu. We can have two calls to Start that both (sequentially) invoke line 284.
• Now assume we have the init process exit, SIGCHLD is received by the shim, and processExits reads, acquires s.lifecycleMu and stores s.containerInitExit[container]. No new execs can actually start after this, because of the check on line 280.
• Then outside s.lifecycleMu handleInitProcess is invoked. We lock s.lifecyleMu, find s.runningExecs[container] is > 0, and make a 2-length channel in s.execCountSubscribers[container].
• Both of the execs call container.Start, invoke runc, and fail because the container is no longer running
• Each one acquires s.lifecycleMu and sends the count of runningExecs to the subscriber on line 299, first sending 1 then 0
• handleInitProcess reads from the channel and finds a 0, and breaks out of the loop
• A new Start call comes in and acquires s.lifecycleMu ahead of handleInitProcess's defer, but fails to make progress because of line 280

I'm struggling to think of a situation where:

• handleInitExit is already in the channel loop (which means both (a) the init process is exited and we've stored it into s.containerInitExit already, and (b) s.runningExecs[container] is at least 1
• Another exec can come in and increment s.runningExecs further after a 0 has been sent to ch

Hmm, the channel created in s.handleInitExits has a buffer of size s.runningExecs[c], and we block new execs from being started before handleInitExits is called, so at most we'll process s.runningExecs[c] exits – so I don't think this would ever block.

It seems feasible that the shim is not even notified that the container init exited until after we have gotten the failure to start the exec here but before we take the lock at L296?

It seems feasible that the shim is not even notified that the container init exited until after we have gotten the failure to start the exec here but before we take the lock at L296?

Following that thread:

• container init is started
• (1) container exec is started
  • we increment s.runningExecs in L284, then start the exec
• (1) the exec start call fails (L290)
• (2) container init exits and s.processExits acquires s.lifecycleMu and processes it
  • init exit gets stashed in s.containerInitExits, and s.handleInitExit gets called (and 2. unlocks s.lifecycleMu)
• (1) acquires the lock at L296, and decrements s.runningExecs
• (2)s.handleInitExit locks s.lifecycleMu checks s.runningExecs, finds 0, and immediately processes the init exit

Or:

• container init is started
• (1) container exec is started
  • we increment s.runningExecs in L284, then start the exec
• (1) the exec start call fails (L290)
• (2) container init exits and s.processExits acquires s.lifecycleMu and processes it
  • init exit gets stashed in s.containerInitExits, and s.handleInitExit gets called (and 2. unlocks s.lifecycleMu)
• (2)s.handleInitExit locks s.lifecycleMu checks s.runningExecs, finds 1, creates a buffered channel with length 1, launches the goroutine and unlocks s.lifecycleMu
• (1) acquires the lock at L296, and decrements s.runningExecs, and sends (a 0) on the channel
• (2) the goroutine receives on the channel, finds a 0, and processes the init exit.

This may be better as a cond var with lifecyclemu to avoid more data races.

There'd still need to be some way for handleInitExit to be notified to check the count. That could be with a chan<- struct{}, but it seems cheaper to send the value over the chan<- int and avoid acquiring s.lifecycleMu if the value is > 0.

Right, I did have a branch somewhere with a sync.Cond, but I ended up going with Cory's suggestion for a map of channels since it seemed simpler/more verifiably correct. In general I think I prefer this approach since it reduces lock contention over s.lifecycleMu inside the goroutine, and we only have to lock it to set the channel and then remove it when the goroutine is done, but let me know if you have a strong opinion/have spotted any race condition in particular!