Skip to content

Commit 13ddd06

Browse files
lpcoxCopilot
andcommitted
review: use RLock in Registry.Close, add shutdown log, test double-close
- Use RLock/RUnlock instead of Lock/Unlock in Registry.Close() since the guards map is only read (not modified) during the closers scan - Add summary LogInfo after closing guards for shutdown visibility - Add double-close idempotency test with closeCount tracking on mock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 64bf699 commit 13ddd06

2 files changed

Lines changed: 20 additions & 4 deletions

File tree

internal/guard/guard_test.go

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ type mockGuard struct {
2121
// mockClosableGuard is a guard that tracks whether Close was called
2222
type mockClosableGuard struct {
2323
mockGuard
24-
closed bool
25-
closeErr error
24+
closed bool
25+
closeCount int
26+
closeErr error
2627
}
2728

2829
func (m *mockClosableGuard) Close(ctx context.Context) error {
2930
m.closed = true
31+
m.closeCount++
3032
return m.closeErr
3133
}
3234

@@ -514,6 +516,17 @@ func TestGuardRegistry_Close(t *testing.T) {
514516
assert.True(t, g1.closed, "expected failing guard Close to be called")
515517
assert.True(t, g2.closed, "expected ok guard Close to be called")
516518
})
519+
520+
t.Run("double close is safe", func(t *testing.T) {
521+
registry := NewRegistry()
522+
g := &mockClosableGuard{mockGuard: mockGuard{id: "wasm"}}
523+
registry.Register("server1", g)
524+
525+
registry.Close(context.Background())
526+
registry.Close(context.Background())
527+
528+
assert.Equal(t, 2, g.closeCount, "Close should be called twice without panic")
529+
})
517530
}
518531

519532
func TestCreateGuard(t *testing.T) {

internal/guard/registry.go

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -111,20 +111,23 @@ func (r *Registry) Close(ctx context.Context) {
111111
c interface{ Close(context.Context) error }
112112
}
113113

114-
r.mu.Lock()
114+
r.mu.RLock()
115115
closers := make([]closableGuard, 0, len(r.guards))
116116
for id, g := range r.guards {
117117
if c, ok := g.(interface{ Close(context.Context) error }); ok {
118118
closers = append(closers, closableGuard{id: id, c: c})
119119
}
120120
}
121-
r.mu.Unlock()
121+
r.mu.RUnlock()
122122

123123
for _, guard := range closers {
124124
if err := guard.c.Close(ctx); err != nil {
125125
logger.LogWarn("guard", "Failed to close guard for server %s: %v", guard.id, err)
126126
}
127127
}
128+
if len(closers) > 0 {
129+
logger.LogInfo("guard", "Closed %d guard(s)", len(closers))
130+
}
128131
}
129132

130133
// GuardFactory is a function that creates a guard instance

0 commit comments

Comments
 (0)