fix🐛: let a drain that finishes on the deadline count as finished

The wait was one select over done and ctx.Done(). Both can be ready when
it runs, select picks at random among ready cases, and so a queue that
drained in the same instant the budget expired was reported as an
overrun about half the times it landed there - often enough to be read
as noise, and pointing at the wrong thing when it was not. core's own
RunShutdown re-checks for this reason; this did not.

The tie-break is now a function taking channels rather than a queue,
which is what lets a test hand it a closed done and an expired ctx
together. That state is the whole of the bug and cannot be arrived at
reliably from the outside; over 1000 iterations the single-select
version fails, and the second look does not.

The test for giving up on the deadline read the call counter straight
after shutdownQueue returned, while Shutdown runs on a goroutine nobody
joins. It passed because the goroutine is scheduled promptly, not
because anything ordered the two. The fake now signals that Shutdown has
been entered and the test waits for it.

Both raised by Copilot on #918.

The first attempt at the tie-break test was wrong and is not what
landed: it asserted that an immediately-returning Shutdown always counts
as drained under an already-expired context, which is not true and
should not be - if the goroutine has not run, nothing has drained. That
test failed, correctly. What is being claimed is narrower: when both are
ready, done wins.

app/other/router/demo_guard_test.go

@@ -5,6 +5,7 @@ import (
 
 	"github.com/gin-gonic/gin"
 	jwt "github.com/go-admin-team/go-admin-core/v2/jwtauth"
+	"github.com/go-admin-team/go-admin-core/v2/sdk/config"
 
 	"go-admin/common/middleware"
 )
@@ -12,13 +13,28 @@ import (
 // registeredRoutes builds the generator's routes on an engine of its own and
 // reports the patterns they were registered under.
 //
+// The mode has to be given rather than inherited, because it now decides what
+// gets registered: a test that leaves it at the zero value would be asking
+// about a mode no deployment runs in, and would pass whether or not the gate
+// works.
+//
+// It is put back before this returns, not at the end of the test. t.Cleanup
+// would leave the mode set for everything the caller does afterwards, so a
+// caller that went on to assert something mode-dependent would be reading a
+// value this helper left behind rather than one it chose. A caller that does
+// want the mode set has to set it, which is visible where it happens.
+//
 // The JWT middleware is a zero value. MiddlewareFunc only closes over the
 // receiver and is never called here - no request is served, the engine is
 // asked what it has - so nothing dereferences it.
-func registeredRoutes(t *testing.T) map[string]bool {
+func registeredRoutes(t *testing.T, mode string) map[string]bool {
 	t.Helper()
 	gin.SetMode(gin.TestMode)
 
+	previous := config.ApplicationConfig.Mode
+	defer func() { config.ApplicationConfig.Mode = previous }()
+	config.ApplicationConfig.Mode = mode
+
 	r := gin.New()
 	v1 := r.Group("/api/v1")
 	sysNoCheckRoleRouter(v1, &jwt.GinJWTMiddleware{})
@@ -38,7 +54,7 @@ func registeredRoutes(t *testing.T) map[string]bool {
 // stops matching anything, demo mode silently starts serving it again, and
 // nothing else would say so.
 func TestEveryRouteDemoModeRefusesStillExists(t *testing.T) {
-	routes := registeredRoutes(t)
+	routes := registeredRoutes(t, "demo")
 	for _, guarded := range middleware.DemoWriteRoutes() {
 		if !routes[guarded] {
 			t.Errorf("demo mode refuses %q, but no route is registered under that pattern - "+
@@ -64,7 +80,7 @@ func TestTheGeneratorsReadOnlyRoutesAreNotRefused(t *testing.T) {
 		"/api/v1/db/tables/page",
 		"/api/v1/db/columns/page",
 	} {
-		if !registeredRoutes(t)[readOnly] {
+		if !registeredRoutes(t, "demo")[readOnly] {
 			t.Fatalf("%s is not registered, so this test is asserting against nothing", readOnly)
 		}
 		if refused[readOnly] {

app/other/router/gen_router.go

@@ -3,15 +3,49 @@ package router
 import (
 	"github.com/gin-gonic/gin"
 	jwt "github.com/go-admin-team/go-admin-core/v2/jwtauth"
+	"github.com/go-admin-team/go-admin-core/v2/sdk/config"
+
 	"go-admin/app/admin/apis"
 	"go-admin/app/other/apis/tools"
 )
 
+// GenWriteRoutesEnabled reports whether the code generator's writing endpoints
+// are registered in this process.
+//
+// Three of the generator's endpoints do not read. /gen/toproject writes seven
+// Go and Vue source files onto this host, one of them under the path
+// gen.frontpath names; /gen/apitofile writes a migration; /gen/todb inserts
+// menus and APIs. All three are GET, all three are listed in CasbinExclude,
+// and AuthCheckRole skips what is on that list - so Enforce never runs for
+// them and any account that can log in may call them. That is a bargain a
+// workstation can make and a deployment cannot.
+//
+// dev is the shipped default and is where the generator is meant to be used.
+// demo keeps them because demo mode already has a better answer than a 404:
+// DemoEvn refuses these three by name and explains itself, which is the thing
+// the demo exists to show. test and prod get nothing.
+//
+// The mode is read once, while the routes are being built. Changing
+// application.mode in a running process adds and removes nothing - a
+// configuration reload rebuilds neither the engine nor its routes.
+//
+// core has constants for dev, test and prod but none for demo, which this
+// repository spells as a literal in common/middleware/demo.go. Both are
+// literals here so that the two read as one set rather than two conventions.
+func GenWriteRoutesEnabled() bool {
+	switch config.ApplicationConfig.Mode {
+	case "dev", "demo":
+		return true
+	default:
+		return false
+	}
+}
+
 func init() {
 	routerCheckRole = append(routerCheckRole, sysNoCheckRoleRouter, registerDBRouter, registerSysTableRouter)
 }
 
-func sysNoCheckRoleRouter(v1 *gin.RouterGroup ,authMiddleware *jwt.GinJWTMiddleware) {
+func sysNoCheckRoleRouter(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMiddleware) {
 	r1 := v1.Group("")
 	{
 		sys := apis.System{}
@@ -22,9 +56,11 @@ func sysNoCheckRoleRouter(v1 *gin.RouterGroup ,authMiddleware *jwt.GinJWTMiddlew
 	{
 		gen := tools.Gen{}
 		r.GET("/gen/preview/:tableId", gen.Preview)
-		r.GET("/gen/toproject/:tableId", gen.GenCode)
-		r.GET("/gen/apitofile/:tableId", gen.GenApiToFile)
-		r.GET("/gen/todb/:tableId", gen.GenMenuAndApi)
+		if GenWriteRoutesEnabled() {
+			r.GET("/gen/toproject/:tableId", gen.GenCode)
+			r.GET("/gen/apitofile/:tableId", gen.GenApiToFile)
+			r.GET("/gen/todb/:tableId", gen.GenMenuAndApi)
+		}
 		sysTable := tools.SysTable{}
 		r.GET("/gen/tabletree", sysTable.GetSysTablesTree)
 	}
@@ -53,4 +89,4 @@ func registerSysTableRouter(v1 *gin.RouterGroup, authMiddleware *jwt.GinJWTMiddl
 			tablesInfo.GET("", sysTable.GetSysTablesInfo)
 		}
 	}
-}
+}

app/other/router/gen_router_test.go

@@ -0,0 +1,151 @@
+package router
+
+import (
+	"testing"
+
+	"github.com/go-admin-team/go-admin-core/v2/sdk/config"
+)
+
+// genWritingRoutes are the three that do not read. They write Go and Vue
+// source onto the host, a migration, and rows in sys_menu.
+var genWritingRoutes = []string{
+	"/api/v1/gen/toproject/:tableId",
+	"/api/v1/gen/apitofile/:tableId",
+	"/api/v1/gen/todb/:tableId",
+}
+
+// genReadingRoutes are the rest of the generator's surface. Gating the three
+// above must not cost any of these: a deployment that cannot list its tables
+// or preview a template has lost the feature, not secured it.
+var genReadingRoutes = []string{
+	"/api/v1/gen/preview/:tableId",
+	"/api/v1/gen/tabletree",
+	"/api/v1/db/tables/page",
+	"/api/v1/db/columns/page",
+}
+
+// The endpoints that write are registered where the mode says development and
+// nowhere else.
+//
+// They are in CasbinExclude, so Enforce never runs for them and any account
+// that can log in may call them. dev is the shipped default and is where the
+// generator is meant to be used; demo keeps them because DemoEvn refuses these
+// three by name and saying so is the thing the demo is for. Everything else,
+// including the empty mode a process gets when nothing set one, is refused by
+// not existing.
+func TestGeneratorWritingRoutesExistOnlyWhereTheModeAllowsIt(t *testing.T) {
+	for _, tc := range []struct {
+		mode     string
+		expected bool
+		why      string
+	}{
+		{"dev", true, "the shipped default, and where the generator is used"},
+		{"demo", true, "registered so demo mode can refuse them by name"},
+		{"test", false, "a deployment, however much it is called a test"},
+		{"prod", false, "a deployment"},
+		{"", false, "no mode configured is not a reason to trust the caller"},
+	} {
+		t.Run("mode="+tc.mode, func(t *testing.T) {
+			routes := registeredRoutes(t, tc.mode)
+			for _, writing := range genWritingRoutes {
+				if got := routes[writing]; got != tc.expected {
+					t.Errorf("mode %q: %s registered = %v, want %v (%s)",
+						tc.mode, writing, got, tc.expected, tc.why)
+				}
+			}
+		})
+	}
+}
+
+// The other direction. Refusing too much is as much of a defect as refusing
+// too little, and the read-only half of the generator is what a demo shows.
+func TestGeneratorReadingRoutesExistInEveryMode(t *testing.T) {
+	for _, mode := range []string{"dev", "demo", "test", "prod", ""} {
+		t.Run("mode="+mode, func(t *testing.T) {
+			routes := registeredRoutes(t, mode)
+			for _, reading := range genReadingRoutes {
+				if !routes[reading] {
+					t.Errorf("mode %q: %s is not registered - the gate took a route that only reads",
+						mode, reading)
+				}
+			}
+		})
+	}
+}
+
+// GenWriteRoutesEnabled is what cmd/api reads to decide whether to warn at
+// start-up. If it and the registration ever disagree, the log says one thing
+// and the engine does another, so they are checked against each other rather
+// than each against a list.
+func TestGenWriteRoutesEnabledAgreesWithWhatWasRegistered(t *testing.T) {
+	for _, mode := range []string{"dev", "demo", "test", "prod", ""} {
+		t.Run("mode="+mode, func(t *testing.T) {
+			routes := registeredRoutes(t, mode)
+
+			// registeredRoutes puts the mode back before it returns, so ask
+			// the predicate under a mode set here - about the same value the
+			// engine was just built under.
+			previous := config.ApplicationConfig.Mode
+			t.Cleanup(func() { config.ApplicationConfig.Mode = previous })
+			config.ApplicationConfig.Mode = mode
+
+			claimed := GenWriteRoutesEnabled()
+			actual := routes["/api/v1/gen/todb/:tableId"]
+			if claimed != actual {
+				t.Errorf("mode %q: GenWriteRoutesEnabled() = %v but the route was registered = %v",
+					mode, claimed, actual)
+			}
+		})
+	}
+}
+
+// The helper restores the mode before it returns, so nothing it was asked
+// about leaks into what the caller does next.
+//
+// Worth a test of its own because the failure is silent: a helper that left
+// the mode set would make every assertion after the call read a value the
+// caller did not choose, and each of those assertions would still pass for as
+// long as the leaked value happened to be the right one.
+func TestRegisteredRoutesRestoresTheModeBeforeReturning(t *testing.T) {
+	const sentinel = "not-a-mode"
+
+	previous := config.ApplicationConfig.Mode
+	t.Cleanup(func() { config.ApplicationConfig.Mode = previous })
+	config.ApplicationConfig.Mode = sentinel
+
+	registeredRoutes(t, "prod")
+
+	if got := config.ApplicationConfig.Mode; got != sentinel {
+		t.Errorf("mode after the helper returned = %q, want %q - it was left set to what "+
+			"the helper was asked about", got, sentinel)
+	}
+}
+
+// Changing the mode after the routes were built unregisters nothing.
+//
+// buildRouter has one call site, in run(), and route registration is not on
+// any phase or reload callback - so a configuration reload moves
+// config.ApplicationConfig.Mode without moving the routes. From that moment
+// GenWriteRoutesEnabled answers about a mode the engine was not built under.
+//
+// That gap is why the start-up warning tells the reader to restart rather than
+// only to change the mode. This pins it: if registration ever becomes dynamic,
+// this test fails and the message it justifies has to be revisited.
+func TestChangingTheModeDoesNotUnregisterWhatWasAlreadyBuilt(t *testing.T) {
+	built := registeredRoutes(t, "dev")
+	if !built["/api/v1/gen/todb/:tableId"] {
+		t.Fatal("built under dev without the writing routes, so this test asserts nothing")
+	}
+
+	previous := config.ApplicationConfig.Mode
+	t.Cleanup(func() { config.ApplicationConfig.Mode = previous })
+	config.ApplicationConfig.Mode = "prod"
+
+	if GenWriteRoutesEnabled() {
+		t.Fatal("the predicate still allows prod, so the disagreement below is not the one meant")
+	}
+	if !built["/api/v1/gen/todb/:tableId"] {
+		t.Error("the route left the engine when the mode changed - registration has become " +
+			"dynamic, and the start-up warning's advice to restart is now wrong")
+	}
+}

cmd/api/gen_warning_test.go

@@ -0,0 +1,38 @@
+package api
+
+import (
+	"testing"
+
+	"github.com/go-admin-team/go-admin-core/v2/sdk/config"
+)
+
+// The warning fires exactly where the generator's writing endpoints are served
+// and nothing else refuses them.
+//
+// dev is the case the warning exists for: it is the shipped default, so it is
+// the mode a deployment that changed nothing is running in. demo serves the
+// routes too, but DemoEvn refuses all three by name, so warning there would
+// describe an exposure that is not there.
+func TestGeneratorWriteRoutesWarningFiresWhereTheExposureIs(t *testing.T) {
+	for _, tc := range []struct {
+		mode string
+		want bool
+		why  string
+	}{
+		{"dev", true, "shipped default, endpoints served and not refused"},
+		{"demo", false, "served, but DemoEvn refuses all three"},
+		{"test", false, "not served"},
+		{"prod", false, "not served"},
+		{"", false, "not served"},
+	} {
+		t.Run("mode="+tc.mode, func(t *testing.T) {
+			previous := config.ApplicationConfig.Mode
+			t.Cleanup(func() { config.ApplicationConfig.Mode = previous })
+			config.ApplicationConfig.Mode = tc.mode
+
+			if got := generatorWriteRoutesNeedWarning(); got != tc.want {
+				t.Errorf("mode %q: warning = %v, want %v (%s)", tc.mode, got, tc.want, tc.why)
+			}
+		})
+	}
+}

cmd/api/server.go

@@ -177,6 +177,7 @@ func run() error {
 		gin.SetMode(gin.ReleaseMode)
 	}
 	buildRouter()
+	reportGeneratorWriteRoutes()
 
 	srv := &http.Server{
 		Addr:         fmt.Sprintf("%s:%d", config.ApplicationConfig.Host, config.ApplicationConfig.Port),
@@ -356,6 +357,39 @@ const (
 	kubernetesGraceSeconds = 30
 )
 
+// reportGeneratorWriteRoutes says whether this process serves the code
+// generator's writing endpoints, and to whom.
+//
+// The endpoints are gated on the mode, and the shipped configuration says dev -
+// so the deployment most likely to be exposed is the one that changed nothing,
+// and the one least likely to go looking. Silence there would leave the gate
+// technically correct and practically useless.
+//
+// Nothing is said in demo mode. The routes are registered, but DemoEvn refuses
+// all three by name, so a warning would describe an exposure that is not there.
+func reportGeneratorWriteRoutes() {
+	if !generatorWriteRoutesNeedWarning() {
+		return
+	}
+	log.Warnf("the code generator's writing endpoints are served in mode %q: "+
+		"/api/v1/gen/{toproject,apitofile,todb} write Go and Vue source onto this host and rows "+
+		"into this database, and they are in CasbinExclude, so any account that can log in may "+
+		"call them. Set application.mode to prod or test on anything that is not a workstation, "+
+		"then restart: these routes were registered at start-up and a configuration reload does "+
+		"not rebuild them.",
+		config.ApplicationConfig.Mode)
+}
+
+// generatorWriteRoutesNeedWarning reports whether there is an exposure to warn
+// about: the endpoints are served, and nothing else is refusing them.
+//
+// Split from the logging so the decision can be tested. A warning nobody can
+// make fire is indistinguishable from no warning at all, and this one exists
+// precisely for the case nobody is looking at.
+func generatorWriteRoutesNeedWarning() bool {
+	return otherrouter.GenWriteRoutesEnabled() && config.ApplicationConfig.Mode != "demo"
+}
+
 // reportShutdownBudget states what a shutdown will spend and whether it fits.
 //
 // The sum is taken from the resolved values, not from the configuration file:

common/storage/initialize.go

@@ -8,10 +8,12 @@
 package storage
 
 import (
+	"context"
 	"log"
 	"sync"
 
 	"github.com/go-admin-team/go-admin-core/v2/captcha"
+	corelog "github.com/go-admin-team/go-admin-core/v2/logger"
 	"github.com/go-admin-team/go-admin-core/v2/sdk"
 	"github.com/go-admin-team/go-admin-core/v2/sdk/config"
 )
@@ -21,6 +23,7 @@ func Setup() {
 	setupCache()
 	setupCaptcha()
 	setupQueue()
+	registerQueueDrain()
 }
 
 func setupCache() {
@@ -41,8 +44,123 @@ var (
 	// shut it down, and counted so a consumer can tell one from the next.
 	installed    interface{ Shutdown() }
 	installedGen uint64
+	// drainRegistered records that the BeforeExit callback is on the runtime,
+	// so that a reload does not add another one.
+	drainRegistered bool
 )
 
+// setShutdown is sdk.Runtime.SetShutdown, indirected so that registering can
+// be observed.
+//
+// It has to be: the runtime does not report how many callbacks a phase holds,
+// and shutdownQueue takes the adapter on its first run, so every registration
+// after the first returns immediately and changes nothing anybody can see. A
+// reload adding one callback per round would therefore be invisible from the
+// outside - which is exactly how it would survive.
+var setShutdown = func(f func(context.Context)) { sdk.Runtime.SetShutdown(f) }
+
+// registerQueueDrain puts shutdownQueue on the BeforeExit phase, once.
+//
+// Setup is one of the callbacks bootstrap.SetupConfig re-runs on every
+// configuration change, so registering from it without a guard would leave one
+// callback per reload - each shutting down the same adapter, each reported
+// separately when the budget runs out.
+//
+// A flag under the existing mutex rather than a sync.Once: the tests in this
+// package already save and restore installed and installedGen to keep one test
+// from deciding what the next one sees, and a sync.Once cannot be put back.
+func registerQueueDrain() {
+	queueMu.Lock()
+	first := !drainRegistered
+	drainRegistered = true
+	queueMu.Unlock()
+
+	if first {
+		setShutdown(shutdownQueue)
+	}
+}
+
+// drainedInTime shuts q down and reports whether it finished before ctx expired.
+func drainedInTime(ctx context.Context, q interface{ Shutdown() }) bool {
+	done := make(chan struct{})
+	go func() {
+		defer close(done)
+		q.Shutdown()
+	}()
+	return finishedBeforeDeadline(ctx, done)
+}
+
+// finishedBeforeDeadline waits for done or for ctx, and resolves a tie in
+// favour of done.
+//
+// The tie is the reason this is a function of its own rather than one select
+// inline. Both channels can be ready when the select runs, select picks at
+// random among ready cases, and so a single look reports an overrun for a
+// drain that completed - about half the times it lands there, which is exactly
+// often enough to be dismissed as noise. core's own RunShutdown re-checks for
+// this reason.
+//
+// Taking channels rather than a queue is what makes it testable: a closed done
+// and an expired ctx can be handed in together, which is the state a race
+// would otherwise have to be caught in.
+func finishedBeforeDeadline(ctx context.Context, done <-chan struct{}) bool {
+	select {
+	case <-done:
+		return true
+	case <-ctx.Done():
+	}
+
+	select {
+	case <-done:
+		return true
+	default:
+		return false
+	}
+}
+
+// shutdownQueue drains the queue this package installed, on the way out.
+//
+// Nothing used to. core's Memory.Shutdown closes the queue and waits for every
+// consumer to finish what it is holding, and the legacy adapter cancels its
+// context and closes the underlying queue - but neither ran at exit, so the
+// process left with the login log, the operation log and the API sync still
+// buffered, and left reporting success.
+//
+// The adapter is read here rather than captured at registration because a
+// reload replaces it. Registration happens once per process; this runs against
+// whatever is current when the signal arrives.
+//
+// Only an adapter this package installed. sdk.Runtime.GetQueueAdapter never
+// returns nil - with no queue section configured the runtime wraps its own
+// fallback queue - so going through that accessor would shut down a queue this
+// package neither built nor started.
+//
+// The adapter is taken, not read: after this the package owns nothing, so a
+// reload arriving mid-shutdown builds a new one instead of being handed a
+// closed one to shut down again. Both implementations tolerate a second
+// Shutdown, so this is about who owns it rather than about a crash.
+func shutdownQueue(ctx context.Context) {
+	queueMu.Lock()
+	q := installed
+	installed = nil
+	queueMu.Unlock()
+
+	if q == nil {
+		return
+	}
+
+	if drainedInTime(ctx, q) {
+		corelog.Info("queue: drained")
+		return
+	}
+	// The wait is what the budget bounds, not the work: Shutdown takes no
+	// context and is still running in that goroutine. Saying so here names what
+	// is being lost, which the generic overrun message cannot.
+	corelog.Warnf("queue: the shutdown budget ran out while the queue was still draining - " +
+		"whatever it had not delivered goes with the process. Raise extend.shutdown.cleanup " +
+		"if this recurs.")
+}
+
 // QueueGeneration reports how many times this package has installed a queue
 // adapter. It changes every time setupQueue builds a new one, which is on
 // every configuration reload, and stays 0 for as long as the configuration has

common/storage/queue_drain_test.go

@@ -0,0 +1,333 @@
+package storage
+
+import (
+	"context"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/go-admin-team/go-admin-core/v2/sdk"
+	"github.com/go-admin-team/go-admin-core/v2/sdk/config"
+	"github.com/go-admin-team/go-admin-core/v2/sdk/runtime"
+	corestorage "github.com/go-admin-team/go-admin-core/v2/storage"
+)
+
+// countingQueue stands in for an installed adapter. Only Shutdown is exercised
+// - the drain callback never publishes or consumes - so the rest of
+// AdapterQueue is deliberately absent: `installed` is typed on Shutdown alone,
+// and widening the fake would only invite it to be used for something else.
+type countingQueue struct {
+	calls atomic.Int32
+	block chan struct{}
+
+	// started is closed on the way into Shutdown, so a test can wait for the
+	// call rather than assume the goroutine that makes it was scheduled. The
+	// caller returns on its own deadline while Shutdown is still running, so
+	// reading calls straight after that return is a race with the increment.
+	startOnce sync.Once
+	started   chan struct{}
+}
+
+func newCountingQueue() *countingQueue {
+	return &countingQueue{started: make(chan struct{})}
+}
+
+func (q *countingQueue) Shutdown() {
+	q.startOnce.Do(func() { close(q.started) })
+	q.calls.Add(1)
+	if q.block != nil {
+		<-q.block
+	}
+}
+
+// waitStarted blocks until Shutdown has been entered, or fails the test.
+func (q *countingQueue) waitStarted(t *testing.T) {
+	t.Helper()
+	select {
+	case <-q.started:
+	case <-time.After(5 * time.Second):
+		t.Fatal("Shutdown was never called")
+	}
+}
+
+// isolate gives the test its own runtime and its own view of what this package
+// has installed, and puts the process-wide state back afterwards.
+//
+// Same isolation as TestSetupBumpsTheQueueGenerationOnEveryReload, plus
+// drainRegistered: it is what stops a reload registering a second callback, so
+// leaving it set would make every later test in this binary see a package that
+// has already registered.
+func isolate(t *testing.T) {
+	t.Helper()
+
+	prevQ, prevC := config.QueueConfig, config.CacheConfig
+	prevRuntime := sdk.Runtime
+	queueMu.Lock()
+	prevInstalled, prevGen, prevRegistered := installed, installedGen, drainRegistered
+	queueMu.Unlock()
+
+	t.Cleanup(func() {
+		config.QueueConfig, config.CacheConfig = prevQ, prevC
+		sdk.Runtime = prevRuntime
+		queueMu.Lock()
+		installed, installedGen, drainRegistered = prevInstalled, prevGen, prevRegistered
+		queueMu.Unlock()
+	})
+
+	sdk.Runtime = runtime.NewConfig()
+	queueMu.Lock()
+	installed, drainRegistered = nil, false
+	queueMu.Unlock()
+}
+
+// setInstalled puts a fake where setupQueue would have left the real adapter.
+//
+// Legitimate because the callback reads `installed` when it runs rather than
+// capturing it at registration - that is the property that lets a reload
+// replace the adapter and still have the right one drained.
+func setInstalled(q interface{ Shutdown() }) {
+	queueMu.Lock()
+	installed = q
+	queueMu.Unlock()
+}
+
+func currentInstalled() interface{ Shutdown() } {
+	queueMu.Lock()
+	defer queueMu.Unlock()
+	return installed
+}
+
+// Issue #911: nothing shut the queue down at exit, so whatever was buffered
+// went with the process.
+//
+// This is the half that matters most - a callback is on BeforeExit and it
+// reaches the adapter this package installed. It says nothing about how many
+// times the callback was registered; see the test below for that.
+func TestSetupPutsTheQueueDrainOnBeforeExit(t *testing.T) {
+	isolate(t)
+	config.CacheConfig = &config.Cache{Memory: struct{}{}}
+	config.QueueConfig = &config.Queue{Memory: &config.QueueMemory{PoolSize: 10}}
+
+	Setup()
+	Setup()
+	Setup()
+
+	q := newCountingQueue()
+	setInstalled(q)
+
+	if err := sdk.Runtime.RunShutdown(context.Background()); err != nil {
+		t.Fatalf("RunShutdown: %v", err)
+	}
+
+	if got := q.calls.Load(); got != 1 {
+		t.Errorf("Shutdown called %d times, want 1 - 0 means nothing registered the drain", got)
+	}
+}
+
+// Setup is re-run on every configuration change, so registering from it has to
+// be guarded: a callback per reload would leave the shutdown phase holding a
+// row of identical entries, each timed and each eligible to be named as the one
+// that overran the budget.
+//
+// Counted at the seam rather than through the effect. The test above cannot see
+// this - shutdownQueue takes the adapter on its first run, so the second and
+// third callbacks find nothing and return, and three registrations produce
+// exactly the same observable result as one. That is a good property of the
+// callback and a blind spot for any test that goes through it.
+func TestSetupRegistersTheDrainOncePerProcessHoweverManyReloads(t *testing.T) {
+	isolate(t)
+
+	previous := setShutdown
+	t.Cleanup(func() { setShutdown = previous })
+	registrations := 0
+	setShutdown = func(func(context.Context)) { registrations++ }
+
+	config.CacheConfig = &config.Cache{Memory: struct{}{}}
+	config.QueueConfig = &config.Queue{Memory: &config.QueueMemory{PoolSize: 10}}
+
+	Setup()
+	Setup()
+	Setup()
+
+	if registrations != 1 {
+		t.Errorf("three reloads registered the drain %d times, want 1", registrations)
+	}
+}
+
+// The drain reaches the adapter that is current when the signal arrives, not
+// one captured while wiring up. A reload replaces the adapter, and draining the
+// one that was installed at start-up would drain something nobody has published
+// to since.
+func TestTheDrainRunsAgainstTheAdapterInstalledLast(t *testing.T) {
+	isolate(t)
+	config.CacheConfig = &config.Cache{Memory: struct{}{}}
+	config.QueueConfig = &config.Queue{Memory: &config.QueueMemory{PoolSize: 10}}
+	Setup()
+
+	first, second := newCountingQueue(), newCountingQueue()
+	setInstalled(first)
+	setInstalled(second)
+
+	if err := sdk.Runtime.RunShutdown(context.Background()); err != nil {
+		t.Fatalf("RunShutdown: %v", err)
+	}
+
+	if first.calls.Load() != 0 {
+		t.Error("the adapter that was replaced was shut down; the callback captured it instead of " +
+			"reading it when it ran")
+	}
+	if second.calls.Load() != 1 {
+		t.Errorf("the current adapter was shut down %d times, want 1", second.calls.Load())
+	}
+}
+
+// Nothing installed is the shipped default: settings.yml has no queue section,
+// so setupQueue returns early and the runtime's own fallback queue is what
+// callers get. Shutting that down would close a queue this package neither
+// built nor started.
+func TestTheDrainDoesNothingWhenThisPackageInstalledNothing(t *testing.T) {
+	isolate(t)
+	config.CacheConfig = &config.Cache{Memory: struct{}{}}
+	config.QueueConfig = &config.Queue{}
+	Setup()
+
+	if currentInstalled() != nil {
+		t.Fatal("an empty queue configuration installed an adapter, so this test asserts nothing")
+	}
+	if err := sdk.Runtime.RunShutdown(context.Background()); err != nil {
+		t.Fatalf("RunShutdown: %v", err)
+	}
+}
+
+// The budget bounds the wait, not the work. A consumer that never finishes must
+// not hold the process past its grace period - SIGKILL would arrive mid-write
+// instead of at a point of the process's choosing.
+func TestTheDrainStopsWaitingWhenTheBudgetIsGone(t *testing.T) {
+	isolate(t)
+
+	blocked := newCountingQueue()
+	blocked.block = make(chan struct{})
+	t.Cleanup(func() { close(blocked.block) })
+	setInstalled(blocked)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+
+	returned := make(chan struct{})
+	go func() {
+		defer close(returned)
+		shutdownQueue(ctx)
+	}()
+
+	select {
+	case <-returned:
+	case <-time.After(5 * time.Second):
+		t.Fatal("shutdownQueue did not return after its context expired - it waits on a Shutdown " +
+			"that takes no context, so the wait has to be bounded here")
+	}
+
+	// Waited for rather than read straight after the return: Shutdown runs on a
+	// goroutine that the caller does not join, so the increment is not ordered
+	// against the caller giving up on its deadline.
+	blocked.waitStarted(t)
+	if blocked.calls.Load() != 1 {
+		t.Errorf("Shutdown called %d times, want 1 - the drain has to be attempted even when it "+
+			"cannot be waited out", blocked.calls.Load())
+	}
+}
+
+// After the drain this package owns nothing. A reload arriving mid-shutdown
+// then builds a new adapter rather than being handed a closed one as its
+// `previous` to shut down again.
+func TestTheDrainGivesUpOwnershipOfTheAdapter(t *testing.T) {
+	isolate(t)
+	setInstalled(newCountingQueue())
+
+	shutdownQueue(context.Background())
+
+	if got := currentInstalled(); got != nil {
+		t.Errorf("installed is %T after the drain, want nil", got)
+	}
+}
+
+// runtimeQueue is a full AdapterQueue, so it can be handed to the runtime
+// rather than only to this package's own record of what it installed.
+type runtimeQueue struct {
+	countingQueue
+}
+
+func (q *runtimeQueue) String() string                            { return "runtime-fake" }
+func (q *runtimeQueue) Append(corestorage.Messager) error         { return nil }
+func (q *runtimeQueue) Register(string, corestorage.ConsumerFunc) {}
+func (q *runtimeQueue) Run()                                      {}
+
+// The drain must not reach a queue this package did not install.
+//
+// sdk.Runtime.GetQueueAdapter never returns nil: with no queue section
+// configured it wraps the runtime's own fallback, and the wrapper's Shutdown
+// forwards. Reaching for the accessor would therefore look like it worked and
+// would close a queue this package neither built nor started - the same shape
+// as the `if q := GetQueueAdapter(); q != nil` that setupQueue already had to
+// drop.
+//
+// The previous test's empty-configuration case cannot see this: it only checks
+// that RunShutdown returns, which it would either way.
+func TestTheDrainNeverReachesTheRuntimesOwnQueue(t *testing.T) {
+	isolate(t)
+
+	onTheRuntime := &runtimeQueue{countingQueue: *newCountingQueue()}
+	sdk.Runtime.SetQueueAdapter(onTheRuntime)
+
+	if currentInstalled() != nil {
+		t.Fatal("this package installed something, so the distinction under test is not set up")
+	}
+	if sdk.Runtime.GetQueueAdapter() == nil {
+		t.Fatal("the accessor returned nil, so it is no longer the trap this guards")
+	}
+
+	shutdownQueue(context.Background())
+
+	if got := onTheRuntime.calls.Load(); got != 0 {
+		t.Errorf("the runtime's queue was shut down %d times - the drain went through "+
+			"GetQueueAdapter instead of the adapter this package installed", got)
+	}
+}
+
+// A drain that finishes in the same instant the budget expires counts as
+// finished.
+//
+// Both channels are ready when the select runs, and select picks at random
+// among ready cases, so a single look reports an overrun for a drain that
+// completed - roughly half the times it lands here. The repetition is what
+// makes that visible: one iteration passes either way.
+func TestATieBetweenTheDeadlineAndTheDrainGoesToTheDrain(t *testing.T) {
+	expired, cancel := context.WithCancel(context.Background())
+	cancel()
+	<-expired.Done()
+
+	done := make(chan struct{})
+	close(done)
+
+	for i := 0; i < 1000; i++ {
+		if !finishedBeforeDeadline(expired, done) {
+			t.Fatalf("iteration %d of 1000: both the deadline and the drain were ready and the "+
+				"deadline won - a drain that completed is being reported as an overrun", i)
+		}
+	}
+}
+
+// The other side of it. A drain that really has not finished has to be
+// reported, or the warning never fires and the tie-break above has quietly
+// turned into "always say it drained".
+func TestADrainThatHasNotFinishedIsReportedAsAnOverrun(t *testing.T) {
+	expired, cancel := context.WithCancel(context.Background())
+	cancel()
+	<-expired.Done()
+
+	stillRunning := make(chan struct{}) // never closed
+
+	if finishedBeforeDeadline(expired, stillRunning) {
+		t.Error("an unfinished drain was reported as having finished in time")
+	}
+}