Skip to content

Commit

Permalink
Raise an error when calling Close multiple times.
Browse files Browse the repository at this point in the history
A non-breaking way of informing informed clients that the client has already been closed.
  • Loading branch information
MattFenelon committed Nov 6, 2013
1 parent b587d38 commit e08407a
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 8 deletions.
14 changes: 9 additions & 5 deletions client.go
@@ -1,6 +1,7 @@
package riakpbc

import (
"errors"
"log"
"time"
)
Expand Down Expand Up @@ -48,6 +49,9 @@ func NewClientWithCoder(cluster []string, coder *Coder) *Client {
//
// Nodes which are down get set to redial in the background.
func (c *Client) Dial() error {
c.closeChannel = make(chan bool)
c.isClosed = false

for _, node := range c.pool.nodes {
err := node.Dial()
if err != nil {
Expand All @@ -68,25 +72,25 @@ func (c *Client) Dial() error {
}

// Close closes the node TCP connections.
func (c *Client) Close() {
func (c *Client) Close() error {
if c.isClosed {
return
return errors.New("Client has been closed.")
}

c.closeChannel <- true
c.isClosed = true
c.pool.Close()
close(c.closeChannel)
return nil
}

func (c *Client) BackgroundNodePing() {
loop := true
for loop {
for {
select {
case <-time.After(time.Duration(c.pingFrequency) * time.Millisecond):
c.pool.Ping()
case <-c.closeChannel:
loop = false
return
}
}
}
Expand Down
6 changes: 3 additions & 3 deletions client_test.go
Expand Up @@ -28,7 +28,7 @@ func TestBackgroundPingDoesNotCausePanicWhenClosed(t *testing.T) {
riak.BackgroundNodePing()
}

func TestCallingCloseMultipleTimesDoesNotCausePanic(t *testing.T) {
func TestCallingCloseOnAClosedClientReturnsAnError(t *testing.T) {
defer func() {
if err := recover(); err != nil {
t.Fatal(err)
Expand All @@ -37,6 +37,6 @@ func TestCallingCloseMultipleTimesDoesNotCausePanic(t *testing.T) {

riak := clientTestSetupSingleNodeConnection(t)
riak.Close()
riak.Close()
riak.Close()
err := riak.Close()
assert.T(t, err != nil)
}

0 comments on commit e08407a

Please sign in to comment.