Why can't lifecycleMu be used to guard pendingExecs? If the new mutex is required, what's the lock hierarchy w.r.t. mu and lifecycleMu? How can we be sure deadlocks can't happen?

I despise boolean flag parameters which alter the behaviour of a function. It's usually a smell that it's really two distinct functions in a trenchcoat. And the flag is not necessary. The handleStarted closure does not care about the flag so all that matters is that the pendingExecs[c.Pid()] is incremented while lifecycleMu is held, before the exec has been launched. That's easy enough: instead of locking lifecycleMu inside preStart, require that callers hold lifecycleMu when calling preStart. Then Start can be written like so:

	s.lifecycleMu.Lock()
	var cinit *runc.Container
	if r.ExecID == "" {
		cinit = container
	} else {
		s.pendingExecsLock.Lock()
		wg, ok := s.pendingExecs[container.Pid()]
		if !ok {
			wg = &sync.WaitGroup{}
			s.pendingExecs[container.Pid()] = wg
		}
		wg.Add(1)
		s.pendingExecsLock.Unlock()
	}
	handleStarted, cleanup := s.preStart(cinit)
	s.lifecycleMu.Unlock()

And to prove my claim that preStart with the flag parameter is really two functions in a trenchcoat:

func (s *service) preStartExec(cinit *runc.Container) (handleStarted func(*runc.Container, process.Process, bool), cleanup func()) {
	s.lifecycleMu.Lock()
	defer s.lifecycleMu.Unlock()
	
	s.pendingExecsLock.Lock()
	wg, ok := s.pendingExecs[container.Pid()]
	if !ok {
		wg = &sync.WaitGroup{}
		s.pendingExecs[container.Pid()] = wg
	}
	wg.Add(1)
	s.pendingExecsLock.Unlock()
	return s.preStart(nil)
}

I'm not entirely sold on the use of goroutines and WaitGroups for this. The handleStarted closure which wakes this goroutine with wg.Done() could instead check that it is the last pending exec for an exited init process and synchronously dispatch the init process' TaskExit event. (Is the use of goroutines and WaitGroups the reason why pendingExecs can't be guarded with lifecycleMu?)

The handleStarted closure knows the init process' PID and is subscribed to exit events already; it could trivially check if the init process had exited while the exec was starting. The only new information it needs to know is whether or not it is handling an early exit for the last remaining exec in the container. Similarly, processExit could use the same information to know to suppress the handleProcessExit call for an init process with outstanding execs. That could be implemented e.g. by adding a counter field to the struct currently named containerProcess. I think it's feasible to fix exit ordering without any new goroutines, mutexes, or service struct fields, by leveraging the existing exitSubscribers and running fields.

The handleStarted closure knows the init process' PID and is subscribed to exit events already; it could trivially check if the init process had exited while the exec was starting

But the problem isn't letting the handleStarted closure check if the init process has exited while the exec is starting, the problem is knowing whether to delay emitting the init process exit or not/know whether there might be pending execs.

Similarly, processExit could use the same information to know to suppress the handleProcessExit call for an init process with outstanding execs. That could be implemented e.g. by adding a counter field to the struct currently named containerProcess.

That might be viable, I guess we'd increment containerProcessPendingExecs before starting and decrement after processing the exec, and we could use that in processExits to know whether we can process that exit or not (is this a WaitGroup with extra steps?).

However, I don't understand how we can do that without any new goroutines, .... We have to launch a goroutine for that init process exit so we can block until it can be processed and continue processing other exits in the meantime, no? Otherwise we have to make severe modifications to this code. Another question is: if we remove the WaitGroup, will we hand craft some implementation using channels/a sync.Cond/whatever to let our waiting init process exits that they should exit, or do something else entirely?

Ohhhh nevermind, I see! I had a vague idea while looking at all of this that there should be a better way but I didn't figure it out, but this sounds viable! I'll try it out.

No, it does not. I'm probably not going to win the argument in this PR, but that won't stop me from reminding everyone in every PR which touches this file.

Oh! I've missed and/or forgot that discussion. Yes, this was something that looked off to me again while working on this but I got distracted with other things and ended up forgetting about it. I'd rather not block this change on that (apparently contentious) topic, but I'd be happy to open another PR so we can revisit this lock.

Why is the delete done after the s.handleProcessExitL? If the pid is still in s.pendingExecs, what stopping this condition from getting hit multiple times?