Rename Shutdown to Close; implement io.Closer.

This commit is contained in:
Alex Bramley 2016-09-16 19:40:27 +01:00
parent 734da36009
commit 82bcd7aded
2 changed files with 24 additions and 23 deletions

View File

@ -386,9 +386,9 @@ func (conn *Conn) send() {
case line := <-conn.out: case line := <-conn.out:
if err := conn.write(line); err != nil { if err := conn.write(line); err != nil {
logging.Error("irc.send(): %s", err.Error()) logging.Error("irc.send(): %s", err.Error())
// We can't defer this, because Shutdown() waits for it. // We can't defer this, because Close() waits for it.
conn.wg.Done() conn.wg.Done()
conn.Shutdown() conn.Close()
return return
} }
case <-conn.die: case <-conn.die:
@ -409,9 +409,9 @@ func (conn *Conn) recv() {
if err != io.EOF { if err != io.EOF {
logging.Error("irc.recv(): %s", err.Error()) logging.Error("irc.recv(): %s", err.Error())
} }
// We can't defer this, because Shutdown() waits for it. // We can't defer this, because Close() waits for it.
conn.wg.Done() conn.wg.Done()
conn.Shutdown() conn.Close()
return return
} }
s = strings.Trim(s, "\r\n") s = strings.Trim(s, "\r\n")
@ -503,20 +503,20 @@ func (conn *Conn) rateLimit(chars int) time.Duration {
return 0 return 0
} }
// Shutdown tears down all connection-related state. It is called when either // Close tears down all connection-related state. It is called when either
// the sending or receiving goroutines encounter an error. // the sending or receiving goroutines encounter an error.
// It may also be used to forcibly shut down the connection to the server. // It may also be used to forcibly shut down the connection to the server.
func (conn *Conn) Shutdown() { func (conn *Conn) Close() error {
// Guard against double-call of Shutdown() if we get an error in send() // Guard against double-call of Close() if we get an error in send()
// as calling sock.Close() will cause recv() to receive EOF in readstring() // as calling sock.Close() will cause recv() to receive EOF in readstring()
conn.mu.Lock() conn.mu.Lock()
if !conn.connected { if !conn.connected {
conn.mu.Unlock() conn.mu.Unlock()
return return nil
} }
logging.Info("irc.Shutdown(): Disconnected from server.") logging.Info("irc.Close(): Disconnected from server.")
conn.connected = false conn.connected = false
conn.sock.Close() err := conn.sock.Close()
close(conn.die) close(conn.die)
// Drain both in and out channels to avoid a deadlock if the buffers // Drain both in and out channels to avoid a deadlock if the buffers
// have filled. See TestSendDeadlockOnFullBuffer in connection_test.go. // have filled. See TestSendDeadlockOnFullBuffer in connection_test.go.
@ -527,6 +527,7 @@ func (conn *Conn) Shutdown() {
// Dispatch after closing connection but before reinit // Dispatch after closing connection but before reinit
// so event handlers can still access state information. // so event handlers can still access state information.
conn.dispatch(&Line{Cmd: DISCONNECTED, Time: time.Now()}) conn.dispatch(&Line{Cmd: DISCONNECTED, Time: time.Now()})
return err
} }
// drainIn sends all data buffered in conn.in to /dev/null. // drainIn sends all data buffered in conn.in to /dev/null.

View File

@ -70,11 +70,11 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
func (s *testState) tearDown() { func (s *testState) tearDown() {
s.nc.ExpectNothing() s.nc.ExpectNothing()
s.c.Shutdown() s.c.Close()
s.ctrl.Finish() s.ctrl.Finish()
} }
// Practically the same as the above test, but Shutdown is called implicitly // Practically the same as the above test, but Close is called implicitly
// by recv() getting an EOF from the mock connection. // by recv() getting an EOF from the mock connection.
func TestEOF(t *testing.T) { func TestEOF(t *testing.T) {
c, s := setUp(t) c, s := setUp(t)
@ -197,7 +197,7 @@ func TestSendExitsOnDie(t *testing.T) {
// Now, use the control channel to exit send and kill the goroutine. // Now, use the control channel to exit send and kill the goroutine.
// This sneakily uses the fact that the other two goroutines that would // This sneakily uses the fact that the other two goroutines that would
// normally be waiting for die to close are not running, so we only send // normally be waiting for die to close are not running, so we only send
// to the goroutine started above. Normally Shutdown() closes c.die and // to the goroutine started above. Normally Close() closes c.die and
// signals to all three goroutines (send, ping, runLoop) to exit. // signals to all three goroutines (send, ping, runLoop) to exit.
exited.assertNotCalled("Exited before signal sent.") exited.assertNotCalled("Exited before signal sent.")
c.die <- struct{}{} c.die <- struct{}{}
@ -230,7 +230,7 @@ func TestSendExitsOnWriteError(t *testing.T) {
s.nc.Expect("SENT AFTER START") s.nc.Expect("SENT AFTER START")
// Now, close the underlying socket to cause write() to return an error. // Now, close the underlying socket to cause write() to return an error.
// This will call Shutdown() => a call to st.Wipe() will happen. // This will call Close() => a call to st.Wipe() will happen.
exited.assertNotCalled("Exited before signal sent.") exited.assertNotCalled("Exited before signal sent.")
s.nc.Close() s.nc.Close()
// Sending more on c.out shouldn't reach the network, but we need to send // Sending more on c.out shouldn't reach the network, but we need to send
@ -244,8 +244,8 @@ func TestSendDeadlockOnFullBuffer(t *testing.T) {
// Passing a second value to setUp stops goroutines from starting // Passing a second value to setUp stops goroutines from starting
c, s := setUp(t, false) c, s := setUp(t, false)
// We can't use tearDown here because we're testing a deadlock condition // We can't use tearDown here because we're testing a deadlock condition
// and if tearDown tries to call Shutdown() it will deadlock some more // and if tearDown tries to call Close() it will deadlock some more
// because send() is holding the conn mutex via Shutdown() already. // because send() is holding the conn mutex via Close() already.
defer s.ctrl.Finish() defer s.ctrl.Finish()
// We want to test that the a goroutine calling send will exit correctly. // We want to test that the a goroutine calling send will exit correctly.
@ -257,7 +257,7 @@ func TestSendDeadlockOnFullBuffer(t *testing.T) {
// The deadlock arises when a handler being called from conn.dispatch() in // The deadlock arises when a handler being called from conn.dispatch() in
// runLoop() tries to write to conn.out to send a message back to the IRC // runLoop() tries to write to conn.out to send a message back to the IRC
// server, but the buffer is full. If at the same time send() is // server, but the buffer is full. If at the same time send() is
// calling conn.Shutdown() and waiting in there for runLoop() to call // calling conn.Close() and waiting in there for runLoop() to call
// conn.wg.Done(), it will not empty the buffer of conn.out => deadlock. // conn.wg.Done(), it will not empty the buffer of conn.out => deadlock.
// //
// We simulate this by artifically filling conn.out. We must use a // We simulate this by artifically filling conn.out. We must use a
@ -282,7 +282,7 @@ func TestSendDeadlockOnFullBuffer(t *testing.T) {
// At this point the handler should be blocked on a write to conn.out, // At this point the handler should be blocked on a write to conn.out,
// preventing runLoop from looping and thus noticing conn.die is closed. // preventing runLoop from looping and thus noticing conn.die is closed.
// //
// The next part is to force send() to call conn.Shutdown(), which can // The next part is to force send() to call conn.Close(), which can
// be done by closing the fake net.Conn so that it returns an error on // be done by closing the fake net.Conn so that it returns an error on
// calls to Write(): // calls to Write():
s.nc.ExpectNothing() s.nc.ExpectNothing()
@ -290,7 +290,7 @@ func TestSendDeadlockOnFullBuffer(t *testing.T) {
// Now when send is started it will read one line from conn.out and try // Now when send is started it will read one line from conn.out and try
// to write it to the socket. It should immediately receive an error and // to write it to the socket. It should immediately receive an error and
// call conn.Shutdown(), triggering the deadlock as it waits forever for // call conn.Close(), triggering the deadlock as it waits forever for
// runLoop to call conn.wg.Done. // runLoop to call conn.wg.Done.
go func() { go func() {
c.send() c.send()
@ -301,8 +301,8 @@ func TestSendDeadlockOnFullBuffer(t *testing.T) {
<-time.After(time.Millisecond) <-time.After(time.Millisecond)
// Verify that the connection no longer thinks it's connected, i.e. // Verify that the connection no longer thinks it's connected, i.e.
// conn.Shutdown() has definitely been called. We can't call // conn.Close() has definitely been called. We can't call
// conn.Connected() here because conn.Shutdown() holds the mutex. // conn.Connected() here because conn.Close() holds the mutex.
if c.connected { if c.connected {
t.Errorf("Conn still thinks it's connected to the server.") t.Errorf("Conn still thinks it's connected to the server.")
} }
@ -441,7 +441,7 @@ func TestPing(t *testing.T) {
// Now kill the ping loop. // Now kill the ping loop.
// This sneakily uses the fact that the other two goroutines that would // This sneakily uses the fact that the other two goroutines that would
// normally be waiting for die to close are not running, so we only send // normally be waiting for die to close are not running, so we only send
// to the goroutine started above. Normally Shutdown() closes c.die and // to the goroutine started above. Normally Close() closes c.die and
// signals to all three goroutines (send, ping, runLoop) to exit. // signals to all three goroutines (send, ping, runLoop) to exit.
exited.assertNotCalled("Exited before signal sent.") exited.assertNotCalled("Exited before signal sent.")
c.die <- struct{}{} c.die <- struct{}{}
@ -493,7 +493,7 @@ func TestRunLoop(t *testing.T) {
// Now, use the control channel to exit send and kill the goroutine. // Now, use the control channel to exit send and kill the goroutine.
// This sneakily uses the fact that the other two goroutines that would // This sneakily uses the fact that the other two goroutines that would
// normally be waiting for die to close are not running, so we only send // normally be waiting for die to close are not running, so we only send
// to the goroutine started above. Normally Shutdown() closes c.die and // to the goroutine started above. Normally Close() closes c.die and
// signals to all three goroutines (send, ping, runLoop) to exit. // signals to all three goroutines (send, ping, runLoop) to exit.
exited.assertNotCalled("Exited before signal sent.") exited.assertNotCalled("Exited before signal sent.")
c.die <- struct{}{} c.die <- struct{}{}