Browse code

Merge pull request #51448 from akerouanton/stop-events-service

daemon: close EventsService on shutdown

Sebastiaan van Stijn authored on 2025/11/10 22:20:28
Showing 4 changed files
... ...
@@ -1493,6 +1493,12 @@ func (daemon *Daemon) Shutdown(ctx context.Context) error {
1493 1493
 		daemon.mdDB.Close()
1494 1494
 	}
1495 1495
 
1496
+	// At this point, everything has been shut down and no containers are
1497
+	// running anymore. If there are still some open connections to the
1498
+	// '/events' endpoint, closing the EventsService should tear them down
1499
+	// immediately.
1500
+	daemon.EventsService.Close()
1501
+
1496 1502
 	return daemon.cleanupMounts(cfg)
1497 1503
 }
1498 1504
 
... ...
@@ -151,3 +151,9 @@ func (e *Events) loadBufferedEvents(since, until time.Time, topic func(any) bool
151 151
 	}
152 152
 	return buffered
153 153
 }
154
+
155
+// Close all the channels returned to event subscribers.
156
+func (e *Events) Close() error {
157
+	e.pub.Close()
158
+	return nil
159
+}
... ...
@@ -353,7 +353,12 @@ func (s *systemRouter) getEvents(ctx context.Context, w http.ResponseWriter, r *
353 353
 
354 354
 	for {
355 355
 		select {
356
-		case ev := <-l:
356
+		case ev, ok := <-l:
357
+			if !ok {
358
+				log.G(ctx).Debug("event channel closed")
359
+				return nil
360
+			}
361
+
357 362
 			jev, ok := ev.(events.Message)
358 363
 			if !ok {
359 364
 				log.G(ctx).Warnf("unexpected event message: %q", ev)
... ...
@@ -6,6 +6,7 @@ import (
6 6
 	"net/netip"
7 7
 	"slices"
8 8
 	"testing"
9
+	"time"
9 10
 
10 11
 	"github.com/google/go-cmp/cmp/cmpopts"
11 12
 	"github.com/moby/moby/api/types/network"
... ...
@@ -518,3 +519,42 @@ func TestSwarmNoNftables(t *testing.T) {
518 518
 		assert.Check(t, is.ErrorContains(err, "daemon exited during startup"))
519 519
 	})
520 520
 }
521
+
522
+// TestDaemonShutsDownQuicklyDespiteEventsConnection checks whether the daemon
523
+// shuts down in less than 5 secs when there's an active API connection to the
524
+// '/events' endpoint.
525
+//
526
+// As this test verifies timing behavior, it may become flaky. Feel free to
527
+// delete it if it's too annoying.
528
+//
529
+// Regression test for https://github.com/moby/moby/issues/32357#issuecomment-3496848492
530
+func TestDaemonShutsDownQuicklyDespiteEventsConnection(t *testing.T) {
531
+	skip.If(t, testEnv.IsRemoteDaemon, "cannot start daemon on remote test run")
532
+	// The Engine is presumably working the same way on Windows and Linux.
533
+	// Avoid running on Windows as it may be more flaky there.
534
+	skip.If(t, testEnv.DaemonInfo.OSType == "windows")
535
+
536
+	t.Parallel()
537
+
538
+	ctx := testutil.StartSpan(baseContext, t)
539
+
540
+	d := daemon.New(t)
541
+	defer d.Cleanup(t)
542
+
543
+	// This is a parallel test, disable iptables integration.
544
+	d.StartWithBusybox(ctx, t, "--iptables=false", "--ip6tables=false")
545
+	defer d.Stop(t)
546
+
547
+	apiClient := d.NewClientT(t)
548
+	// Open a connection to the '/events' endpoint.
549
+	apiClient.Events(ctx, client.EventsListOptions{})
550
+
551
+	// Kill the daemon
552
+	t0 := time.Now()
553
+	d.Stop(t)
554
+
555
+	dt := time.Since(t0)
556
+	if dt.Seconds() > 5 {
557
+		t.Error("the daemon took more than 5 secs to shutdown")
558
+	}
559
+}