Synchronize notifications and client gr shutdown.

The responses chan for a websocket client was being closed by one of
the websocket goroutines, but it was not the only sender to this
channel.  There was also the notification handler, run by the server
to handle notifications to all websocket clients.  It was possible to
hit cases where sends to this channel would still occur (the select
statement doesn't guarantee that the picked channel operation won't
panic, even if there's another that won't).  To fix this, wait on the
client being removed from the notification group, or if the server is
already shutting down, wait on the notification handler completely
closing, to ensure that no more sends to the channel will occur,
before closing the channel.

Fixes #110.
This commit is contained in:
Josh Rickmar 2014-07-14 09:24:41 -05:00
parent b42ab5b743
commit 7ca16dfe70

View file

@ -218,9 +218,10 @@ type rpcServer struct {
requests chan handlerJob requests chan handlerJob
addWSClient chan *websocketClient addWSClient chan *websocketClient
removeWSClient chan *websocketClient removeWSClient chan *websocketClient
broadcasts chan []byte broadcasts chan []byte
notificationHandlerQuit chan struct{}
quit chan struct{} quit chan struct{}
} }
@ -239,11 +240,12 @@ func newRPCServer(listenAddrs []string, maxClients, maxWebsockets int64) (*rpcSe
// Allow all origins. // Allow all origins.
CheckOrigin: func(r *http.Request) bool { return true }, CheckOrigin: func(r *http.Request) bool { return true },
}, },
requests: make(chan handlerJob), requests: make(chan handlerJob),
addWSClient: make(chan *websocketClient), addWSClient: make(chan *websocketClient),
removeWSClient: make(chan *websocketClient), removeWSClient: make(chan *websocketClient),
broadcasts: make(chan []byte), broadcasts: make(chan []byte),
quit: make(chan struct{}), notificationHandlerQuit: make(chan struct{}),
quit: make(chan struct{}),
} }
// Check for existence of cert file and key file // Check for existence of cert file and key file
@ -742,6 +744,17 @@ out:
break out break out
} }
} }
// Remove websocket client from notification group, or if the server is
// shutting down, wait until the notification handler has finished
// running. This is needed to ensure that no more notifications will be
// sent to the client's responses chan before it's closed below.
select {
case s.removeWSClient <- wsc:
case <-s.quit:
<-s.notificationHandlerQuit
}
close(wsc.responses) close(wsc.responses)
s.wg.Done() s.wg.Done()
} }
@ -939,6 +952,7 @@ out:
break out break out
} }
} }
close(s.notificationHandlerQuit)
s.wg.Done() s.wg.Done()
} }