Fix race condition in TestSocketCleanup by protecting listener access with mutex
Fixes bd-160 The race was between Start() writing s.listener and Stop() reading it. Now all listener access is protected by the server mutex: - Start() stores listener under lock after creation - Accept loop reads listener under RLock - Stop() closes listener under lock All RPC tests now pass with -race flag.
This commit is contained in:
@@ -192,11 +192,32 @@ func TestSocketCleanup(t *testing.T) {
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
defer cancel()
|
||||
|
||||
go server.Start(ctx)
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
// Start server in goroutine
|
||||
started := make(chan error, 1)
|
||||
go func() {
|
||||
err := server.Start(ctx)
|
||||
if err != nil {
|
||||
started <- err
|
||||
}
|
||||
}()
|
||||
|
||||
if _, err := os.Stat(socketPath); os.IsNotExist(err) {
|
||||
t.Fatal("Socket file not created")
|
||||
// Wait for socket to be created (with timeout)
|
||||
timeout := time.After(5 * time.Second)
|
||||
ticker := time.NewTicker(10 * time.Millisecond)
|
||||
defer ticker.Stop()
|
||||
|
||||
socketReady := false
|
||||
for !socketReady {
|
||||
select {
|
||||
case err := <-started:
|
||||
t.Fatalf("Server failed to start: %v", err)
|
||||
case <-timeout:
|
||||
t.Fatal("Timeout waiting for socket creation")
|
||||
case <-ticker.C:
|
||||
if _, err := os.Stat(socketPath); err == nil {
|
||||
socketReady = true
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
if err := server.Stop(); err != nil {
|
||||
|
||||
@@ -94,23 +94,33 @@ func (s *Server) Start(ctx context.Context) error {
|
||||
return fmt.Errorf("failed to remove old socket: %w", err)
|
||||
}
|
||||
|
||||
var err error
|
||||
s.listener, err = net.Listen("unix", s.socketPath)
|
||||
listener, err := net.Listen("unix", s.socketPath)
|
||||
if err != nil {
|
||||
return fmt.Errorf("failed to listen on socket: %w", err)
|
||||
}
|
||||
|
||||
// Set socket permissions to 0600 for security (owner only)
|
||||
if err := os.Chmod(s.socketPath, 0600); err != nil {
|
||||
s.listener.Close()
|
||||
listener.Close()
|
||||
return fmt.Errorf("failed to set socket permissions: %w", err)
|
||||
}
|
||||
|
||||
// Store listener under lock
|
||||
s.mu.Lock()
|
||||
s.listener = listener
|
||||
s.mu.Unlock()
|
||||
|
||||
go s.handleSignals()
|
||||
go s.runCleanupLoop()
|
||||
|
||||
// Accept connections using listener
|
||||
for {
|
||||
conn, err := s.listener.Accept()
|
||||
// Get listener under lock
|
||||
s.mu.RLock()
|
||||
listener := s.listener
|
||||
s.mu.RUnlock()
|
||||
|
||||
conn, err := listener.Accept()
|
||||
if err != nil {
|
||||
s.mu.Lock()
|
||||
shutdown := s.shutdown
|
||||
@@ -152,8 +162,14 @@ func (s *Server) Stop() error {
|
||||
}
|
||||
}
|
||||
|
||||
if s.listener != nil {
|
||||
if closeErr := s.listener.Close(); closeErr != nil {
|
||||
// Close listener under lock
|
||||
s.mu.Lock()
|
||||
listener := s.listener
|
||||
s.listener = nil
|
||||
s.mu.Unlock()
|
||||
|
||||
if listener != nil {
|
||||
if closeErr := listener.Close(); closeErr != nil {
|
||||
err = fmt.Errorf("failed to close listener: %w", closeErr)
|
||||
return
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user