fix(up): catch a late starter on a graceful teardown too, not just on-exit

glours' review on this PR (pullrequestreview-5277692679): stopLateStarter
only ever got armed inside stopOnFirstExit's own closure, so it caught a
service racing the on-exit cascade's sweep but not one racing
gracefulTeardown's (Ctrl+C/SIGTERM) identical one-shot stopApplication
sweep -- same root cause, same file, just the other trigger. Worse for
that path: Ctrl+C can land at t~=0, before monitor.Start even starts and
while s.start() (its own uncancelable context) is still climbing the
dependency graph, so most services may still be created, not running.
The only recovery today is a second manual Ctrl+C.

u.isTerminated already flags "termination is underway" and was already
set by gracefulTeardown (just not consulted for late-starter catching,
and set after stopApplication rather than before it). Extract the
late-starter watch out of stopOnFirstExit into its own listener,
stopLateStarters, armed on u.isTerminated regardless of which path set
it, and register it unconditionally (Ctrl+C works regardless of the
on-exit policy, unlike stopOnFirstExit's own listener). stopOnFirstExit
now sets isTerminated itself before its sweep, symmetric with
gracefulTeardown (also reordered to set it before stopApplication, not
after).

isLateStarter is extracted as a pure predicate, following the existing
shouldFollowStartEvent precedent, and unit-tested (TestIsLateStarter):
the previous fix (ca867482a) shipped only against a flaky test hitting
the on-exit case by chance, with no dedicated regression test at all.

Signed-off-by: Nicolas De Loof <[email protected]>

pkg/compose/up.go

@@ -161,6 +161,12 @@ func (s *composeService) runInteractiveUp(ctx context.Context, project *types.Pr
 	}
 	monitor.withListener(u.printer.HandleEvent)
 
+	// Termination -- on-exit cascade or a graceful Ctrl+C/SIGTERM teardown --
+	// stops the application via a one-shot listing (see stopApplication):
+	// registered unconditionally, regardless of the on-exit policy, since a
+	// graceful teardown always runs it and a container whose start was
+	// already in flight when that listing happened comes up after it.
+	monitor.withListener(u.stopLateStarters())
 	if options.Start.OnExit != api.CascadeIgnore {
 		monitor.withListener(u.stopOnFirstExit())
 	}
@@ -276,8 +282,10 @@ func (u *upSession) runEventLoop(ctx context.Context, kEvents <-chan keyboard.Ke
 	gracefulTeardown := func() {
 		first = false
 		u.events.On(newEvent(api.ResourceCompose, api.Working, api.StatusStopping, "Gracefully Stopping... press Ctrl+C again to force"))
-		u.stopApplication()
+		// set before stopApplication's listing so stopLateStarters is armed
+		// no later than the sweep it must catch stragglers for.
 		u.isTerminated.Store(true)
+		u.stopApplication()
 	}
 
 	for {
@@ -344,15 +352,6 @@ func (u *upSession) stopOnFirstExit() api.ContainerEventListener {
 	once := true
 	return func(event api.ContainerEvent) {
 		if !once {
-			// The abort races the start phase, which runs on a deliberately
-			// uncancelable context (SIGTERM management): a container whose
-			// start was in flight when the application was swept comes up
-			// AFTER the stop, and would keep the session alive until its
-			// natural end. The events stream reveals such late starters —
-			// stop each one as it appears.
-			if event.Type == api.ContainerEventStarted {
-				u.stopLateStarter(event.Service)
-			}
 			return
 		}
 		if event.Type != api.ContainerEventExited {
@@ -364,12 +363,41 @@ func (u *upSession) stopOnFirstExit() api.ContainerEventListener {
 		once = false
 		u.exitCode = event.ExitCode
 		u.events.On(newEvent(api.ResourceCompose, api.Working, api.StatusStopping, "Aborting on container exit..."))
+		// set before stopApplication's listing so stopLateStarters is armed
+		// no later than the sweep it must catch stragglers for.
+		u.isTerminated.Store(true)
 		u.stopApplication()
 	}
 }
 
-// stopLateStarter stops one service started after the on-exit abort swept the
-// application — see stopOnFirstExit.
+// stopLateStarters stops any service that starts after termination has begun
+// — the on-exit cascade above or a graceful Ctrl+C/SIGTERM teardown
+// (runEventLoop) — both of which stop the application via stopApplication's
+// one-shot listing. Either start phase (the initial one, or one still
+// climbing the dependency graph on its deliberately uncancelable context)
+// can race that listing: a container whose start was already in flight comes
+// up after the sweep and would otherwise keep the session alive until its
+// natural end. The events stream reveals such late starters — stop each one
+// as it appears, for as long as termination is underway.
+func (u *upSession) stopLateStarters() api.ContainerEventListener {
+	return func(event api.ContainerEvent) {
+		if !isLateStarter(event, u.isTerminated.Load()) {
+			return
+		}
+		u.stopLateStarter(event.Service)
+	}
+}
+
+// isLateStarter reports whether event is a container starting after
+// termination has begun — the on-exit cascade or a graceful Ctrl+C/SIGTERM
+// teardown, either of which sets terminated true before its one-shot stop
+// listing — and so must be caught and stopped: see stopLateStarters.
+func isLateStarter(event api.ContainerEvent, terminated bool) bool {
+	return terminated && event.Type == api.ContainerEventStarted
+}
+
+// stopLateStarter stops one service started after termination swept the
+// application — see stopLateStarters.
 func (u *upSession) stopLateStarter(service string) {
 	u.eg.Go(func() error {
 		err := u.stop(context.WithoutCancel(u.globalCtx), u.project.Name, api.StopOptions{

pkg/compose/up_test.go

@@ -106,6 +106,48 @@ func TestShouldFollowStartEvent(t *testing.T) {
 	}
 }
 
+// TestIsLateStarter is a follow-up to #14140's on-exit-only fix (glours'
+// review on that PR): stopOnFirstExit's own sweep isn't the only path that
+// can race a service still climbing the dependency graph on its
+// uncancelable context -- a graceful Ctrl+C/SIGTERM teardown does too, and
+// arrives with u.isTerminated already true regardless of which path set it.
+// isLateStarter must gate on that shared flag, not on which listener
+// happened to trigger termination.
+func TestIsLateStarter(t *testing.T) {
+	tests := []struct {
+		name       string
+		event      api.ContainerEvent
+		terminated bool
+		want       bool
+	}{
+		{
+			name:       "a container starting before termination is not a late starter",
+			event:      api.ContainerEvent{Type: api.ContainerEventStarted},
+			terminated: false,
+			want:       false,
+		},
+		{
+			name:       "a non-start event after termination is not a late starter",
+			event:      api.ContainerEvent{Type: api.ContainerEventExited},
+			terminated: true,
+			want:       false,
+		},
+		{
+			name:       "a container starting after termination is a late starter",
+			event:      api.ContainerEvent{Type: api.ContainerEventStarted},
+			terminated: true,
+			want:       true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := isLateStarter(tt.event, tt.terminated)
+			assert.Equal(t, got, tt.want)
+		})
+	}
+}
+
 // TestAppendErrDropsCancellationAfterShutdown is the #13985 follow-up: once
 // our own shutdown has canceled globalCtx (monitor detecting termination,
 // SIGINT/SIGTERM, ...), a lingering goroutine (log/attach streaming) racing