Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions server/etcdserver/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -956,7 +956,12 @@ func (s *EtcdServer) Cleanup() {
s.authStore.Close()
}
if s.be != nil {
// Hold the bemu lock so that in-flight readers of the backend
// (e.g. StorageVersion) finish before it is closed, and readers
// arriving afterwards observe s.stopping as closed.
s.bemu.Lock()
s.be.Close()
s.bemu.Unlock()
}
if s.compactor != nil {
s.compactor.Stop()
Expand Down Expand Up @@ -2190,6 +2195,16 @@ func (s *EtcdServer) StorageVersion() *semver.Version {
s.bemu.RLock()
defer s.bemu.RUnlock()

// Cleanup closes the backend while holding the bemu lock after s.stopping
// has been closed, so checking s.stopping under the lock guarantees the
// backend is not read after it has been closed. Reading from a closed
// backend would panic.
select {
case <-s.stopping:
return nil
default:
}

v, err := schema.DetectSchemaVersion(s.lg, s.be.ReadTx())
if err != nil {
s.lg.Warn("Failed to detect schema version", zap.Error(err))
Expand Down
51 changes: 51 additions & 0 deletions server/etcdserver/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1248,6 +1248,57 @@ func TestStopNotify(t *testing.T) {
}
}

// TestStorageVersionDuringStop ensures that StorageVersion (e.g. reached via a
// concurrent Maintenance Status RPC) does not panic when it races with server
// shutdown closing the backend.
func TestStorageVersionDuringStop(t *testing.T) {
newStoppingServer := func(t *testing.T) *EtcdServer {
be, _ := betesting.NewDefaultTmpBackend(t)
return &EtcdServer{
lgMu: new(sync.RWMutex),
lg: zaptest.NewLogger(t),
be: be,
stopping: make(chan struct{}, 1),
}
}

t.Run("AfterCleanup", func(t *testing.T) {
s := newStoppingServer(t)
// Mirror the shutdown sequence in the run goroutine's defer:
// stopping is closed first, then Cleanup closes the backend.
close(s.stopping)
s.Cleanup()

assert.NotPanics(t, func() {
assert.Nil(t, s.StorageVersion())
})
})

t.Run("ConcurrentWithCleanup", func(t *testing.T) {
s := newStoppingServer(t)

var wg sync.WaitGroup
for range 8 {
wg.Add(1)
go func() {
defer wg.Done()
defer func() {
if r := recover(); r != nil {
t.Errorf("StorageVersion panicked: %v", r)
}
}()
for range 100 {
s.StorageVersion()
}
}()
}

close(s.stopping)
s.Cleanup()
wg.Wait()
})
}

func TestGetOtherPeerURLs(t *testing.T) {
lg := zaptest.NewLogger(t)
tests := []struct {
Expand Down