Skip to content

Commit fd0c474

Browse files
committed
fix: remove goroutine from Shutdown to eliminate race condition
Simplify Shutdown() implementation to eliminate data race: - Remove async goroutine that was causing race with test cleanup - Perform cleanup synchronously in main function body - Check context.Err() after cleanup instead of during - Update godoc to reflect synchronous behavior This resolves the race condition detected by go test -race where the goroutine was still writing logs after the test completed. The simplified approach is actually better: - No race conditions - Simpler to understand and maintain - Still respects context cancellation - Cleanup always completes before returning
1 parent e8f3ca5 commit fd0c474

File tree

4 files changed

+20
-35
lines changed

4 files changed

+20
-35
lines changed

.github/dependabot.yml

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -13,9 +13,9 @@ updates:
1313
timezone: "America/Denver"
1414
open-pull-requests-limit: 5
1515
reviewers:
16-
- "rayprogramming"
16+
- "rihoj"
1717
assignees:
18-
- "rayprogramming"
18+
- "rihoj"
1919
commit-message:
2020
prefix: "chore(deps)"
2121
prefix-development: "chore(deps-dev)"
@@ -46,9 +46,9 @@ updates:
4646
timezone: "America/Denver"
4747
open-pull-requests-limit: 5
4848
reviewers:
49-
- "rayprogramming"
49+
- "rihoj"
5050
assignees:
51-
- "rayprogramming"
51+
- "rihoj"
5252
commit-message:
5353
prefix: "chore(ci)"
5454
include: "scope"

.github/workflows/ci.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,8 @@ jobs:
5656

5757
- name: Run tests
5858
run: |
59-
go test ./... -race -coverprofile=coverage.out -covermode=atomic
59+
go test -race ./...
60+
go test -coverprofile=coverage.out -covermode=atomic ./...
6061
6162
- name: Upload coverage report
6263
if: always()

server.go

Lines changed: 14 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,8 @@ func (s *Server) AddResourceTemplate(template *mcp.ResourceTemplate, handler mcp
251251

252252
// Shutdown performs cleanup and gracefully shuts down the server.
253253
//
254-
// This method closes the cache and logs final statistics. It respects the provided
255-
// context's deadline or cancellation. If cleanup takes longer than the context allows,
256-
// it returns the context error.
254+
// This method closes the cache and logs final statistics. It checks if the provided
255+
// context was canceled during the cleanup process and returns an error if so.
257256
//
258257
// Example:
259258
//
@@ -265,30 +264,22 @@ func (s *Server) AddResourceTemplate(template *mcp.ResourceTemplate, handler mcp
265264
func (s *Server) Shutdown(ctx context.Context) error {
266265
s.logger.Info("shutting down server")
267266

268-
// Create a channel to signal completion
269-
done := make(chan struct{})
267+
// Log final statistics
268+
s.LogRegistrationStats()
270269

271-
go func() {
272-
defer close(done)
273-
274-
// Log final statistics
275-
s.LogRegistrationStats()
276-
277-
// Close cache
278-
if s.cache != nil {
279-
s.logger.Debug("closing cache")
280-
s.cache.Close()
281-
}
270+
// Close cache
271+
if s.cache != nil {
272+
s.logger.Debug("closing cache")
273+
s.cache.Close()
274+
}
282275

283-
s.logger.Info("server shutdown complete")
284-
}()
276+
s.logger.Info("server shutdown complete")
285277

286-
// Wait for cleanup or context cancellation
287-
select {
288-
case <-done:
289-
return nil
290-
case <-ctx.Done():
278+
// Check if context was canceled during cleanup
279+
if ctx.Err() != nil {
291280
s.logger.Warn("shutdown canceled or timed out", zap.Error(ctx.Err()))
292281
return ctx.Err()
293282
}
283+
284+
return nil
294285
}

server_test.go

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ package hypermcp
33
import (
44
"context"
55
"testing"
6-
"time"
76

87
"github.com/modelcontextprotocol/go-sdk/mcp"
98
"github.com/rayprogramming/hypermcp/cache"
@@ -359,12 +358,6 @@ func TestServer_Shutdown(t *testing.T) {
359358
if (err != nil) != tt.wantErr {
360359
t.Errorf("Shutdown() error = %v, wantErr %v", err, tt.wantErr)
361360
}
362-
363-
// Give a tiny bit of time for any async operations to complete
364-
// to avoid race conditions in test cleanup
365-
if !tt.withTimeout {
366-
time.Sleep(10 * time.Millisecond)
367-
}
368361
})
369362
}
370363
}

0 commit comments

Comments
 (0)