From 1a23feb53e1b3426258edd87026624b8ba5b831a Mon Sep 17 00:00:00 2001 From: Josh Rickmar Date: Mon, 7 Jul 2014 17:48:38 -0500 Subject: [PATCH] Fix hang on new request receives after shutdown. Previously, requests could still be sent to a shutdown client and added to the client's internal data structures, without ever responding to the future with an error for a shutdown client (causing hangs when blocking on the future receive). This change fixes this by performing a non-blocking read of the client's shutdown channel before adding a request, and responding with the shutdown error if the client has begun or completed its shutdown. ok @davecgh --- infrastructure.go | 28 ++++++++++++++++++++++++---- 1 file changed, 24 insertions(+), 4 deletions(-) diff --git a/infrastructure.go b/infrastructure.go index f6d6692c..02b57665 100644 --- a/infrastructure.go +++ b/infrastructure.go @@ -145,17 +145,33 @@ func (c *Client) NextID() uint64 { } // addRequest associates the passed jsonRequest with the passed id. This allows -// the response from the remote server to be unmarshalled to the appropriate -// type and sent to the specified channel when it is received. +// the response from the remote server to be unmarshalled to the appropiate type +// and sent to the specified channel when it is received. +// +// If the client has already begun shutting down, ErrClientShutdown is returned +// and the request is not added. // // This function is safe for concurrent access. -func (c *Client) addRequest(id uint64, request *jsonRequest) { +func (c *Client) addRequest(id uint64, request *jsonRequest) error { c.requestLock.Lock() defer c.requestLock.Unlock() + // A non-blocking read of the shutdown channel with the request lock + // held avoids adding the request to the client's internal data + // structures if the client is in the process of shutting down (and + // has not yet grabbed the request lock), or has finished shutdown + // already (responding to each outstanding request with + // ErrClientShutdown). + select { + case <-c.shutdown: + return ErrClientShutdown + default: + } + // TODO(davec): Already there? element := c.requestList.PushBack(request) c.requestMap[id] = element + return nil } // removeRequest returns and removes the jsonRequest which contains the response @@ -779,10 +795,14 @@ func (c *Client) sendCmd(cmd btcjson.Cmd) chan *response { return responseChan } - c.addRequest(cmd.Id().(uint64), &jsonRequest{ + err := c.addRequest(cmd.Id().(uint64), &jsonRequest{ cmd: cmd, responseChan: responseChan, }) + if err != nil { + responseChan <- &response{err: err} + return responseChan + } c.marshalAndSend(cmd, responseChan) return responseChan }