Skip to content

Commit

Permalink
♻️ Simplify stdio server shutdown using only context
Browse files Browse the repository at this point in the history
  • Loading branch information
wesen committed Jan 21, 2025
1 parent 38af046 commit 16e523a
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 23 deletions.
20 changes: 19 additions & 1 deletion changelog.md
Original file line number Diff line number Diff line change
Expand Up @@ -149,4 +149,22 @@ Enhanced process group and signal handling in stdio transport:
- Send signals to entire process group instead of just the main process
- Added fallback to direct process signals if process group handling fails
- Improved debug logging for process and signal management
- Fixed issue with signals not being properly received by the server
- Fixed issue with signals not being properly received by the server

# Fixed Channel Close Race Condition

Fixed a race condition in the stdio server where the done channel could be closed multiple times:

- Added sync.Once to ensure done channel is only closed once
- Fixed potential panic during server shutdown
- Improved shutdown coordination between signal handling and Stop method
- Enhanced logging around channel closing operations

# Simplified Server Shutdown

Simplified stdio server shutdown by using only context-based coordination:

- Removed redundant done channel in favor of context cancellation
- Added dedicated scanner context for cleaner shutdown
- Simplified shutdown logic and error handling
- Improved logging messages for shutdown events
30 changes: 8 additions & 22 deletions pkg/server/transports/stdio/stdio.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@ type Server struct {
resourceService services.ResourceService
toolService services.ToolService
initializeService services.InitializeService
done chan struct{}
signalChan chan os.Signal
}

Expand Down Expand Up @@ -54,7 +53,6 @@ func NewServer(logger zerolog.Logger, ps services.PromptService, rs services.Res
resourceService: rs,
toolService: ts,
initializeService: is,
done: make(chan struct{}),
signalChan: make(chan os.Signal, 1),
}
}
Expand All @@ -70,19 +68,17 @@ func (s *Server) Start(ctx context.Context) error {
// Create a channel for scanner errors
scanErrChan := make(chan error, 1)

// Create a cancellable context for the scanner
scannerCtx, cancelScanner := context.WithCancel(ctx)
defer cancelScanner()

// Start scanning in a goroutine
go func() {
for s.scanner.Scan() {
select {
case <-s.done:
s.logger.Debug().Msg("Received done signal, stopping scanner")
scanErrChan <- nil
return
case <-ctx.Done():
s.logger.Debug().
Err(ctx.Err()).
Msg("Context cancelled, stopping scanner")
scanErrChan <- ctx.Err()
case <-scannerCtx.Done():
s.logger.Debug().Msg("Context cancelled, stopping scanner")
scanErrChan <- scannerCtx.Err()
return
default:
line := s.scanner.Text()
Expand Down Expand Up @@ -114,13 +110,12 @@ func (s *Server) Start(ctx context.Context) error {
s.logger.Debug().
Str("signal", sig.String()).
Msg("Received signal in stdio server")
close(s.done)
cancelScanner()
return nil
case <-ctx.Done():
s.logger.Debug().
Err(ctx.Err()).
Msg("Context cancelled in stdio server")
close(s.done)
return ctx.Err()
case err := <-scanErrChan:
if err == io.EOF {
Expand All @@ -138,15 +133,6 @@ func (s *Server) Start(ctx context.Context) error {
func (s *Server) Stop(ctx context.Context) error {
s.logger.Info().Msg("Stopping stdio server")

select {
case <-s.done:
s.logger.Debug().Msg("Server already stopped")
return nil
default:
s.logger.Debug().Msg("Closing done channel to signal shutdown")
close(s.done)
}

// Wait for context to be done or timeout
select {
case <-ctx.Done():
Expand Down

0 comments on commit 16e523a

Please sign in to comment.