Skip to content

Commit

Permalink
server: avoid leaking tenant stopper
Browse files Browse the repository at this point in the history
We've seen at least one stopper leak during a stress run. I was not
able to reproduce it locally, but from the code it seems possible that
we are leaking it here if something like this happens.

1. serverController's scanner ticks and starts scanning for expected
   tenants.

2. The server's stopper stops.

3. The tenant stopper for some tenant, foo, is also stopped in
   response. This tenant is stopped and removed from the running tenant
   entries.

4. Back in the serverController, we see that tenant foo isn't
   started. We try to start it.

5. We make a new stopper, but can't run an async task, so we exit with
   an error, leaving the stopper.

Epic: none

Release note: None
  • Loading branch information
stevendanna committed Feb 13, 2023
1 parent fccc653 commit 9e24c02
Showing 1 changed file with 5 additions and 0 deletions.
5 changes: 5 additions & 0 deletions pkg/server/server_controller_orchestration.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,7 @@ func (c *serverController) startControlledServer(
tenantCtx := logtags.AddTag(context.Background(), "tenant-orchestration", nil)

tenantStopper := stop.NewStopper()

// Ensure that if the surrounding server requests shutdown, we
// propagate it to the new server.
if err := c.stopper.RunAsyncTask(ctx, "propagate-close", func(ctx context.Context) {
Expand All @@ -233,6 +234,10 @@ func (c *serverController) startControlledServer(
tenantStopper.Stop(tenantCtx)
}
}); err != nil {
// The goroutine above is responsible for stopping the
// tenantStopper. If it fails to stop, we stop it here
// to avoid leaking the stopper.
tenantStopper.Stop(ctx)
return nil, err
}

Expand Down

0 comments on commit 9e24c02

Please sign in to comment.