From cd24432da4842f9f4fbc9137e9c248db28e1f6f9 Mon Sep 17 00:00:00 2001
From: Alex Bramley <a.bramley@gmail.com>
Date: Mon, 13 Apr 2015 19:05:42 +0100
Subject: [PATCH] More deadlock fixes for #58.

 - Remove lock in Me().
 - Move call to initialise() to Connect().
 - Ensure DISCONNECTED event is fired after shutdown() lock is released.
---
 client/connection.go      | 12 ++++--------
 client/connection_test.go |  5 +----
 2 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/client/connection.go b/client/connection.go
index 5b683e9..852db07 100644
--- a/client/connection.go
+++ b/client/connection.go
@@ -154,7 +154,6 @@ func Client(cfg *Config) *Conn {
 		lastsent:    time.Now(),
 	}
 	conn.addIntHandlers()
-	conn.initialise()
 	return conn
 }
 
@@ -169,8 +168,6 @@ func (conn *Conn) Config() *Config {
 }
 
 func (conn *Conn) Me() *state.Nick {
-	conn.mu.RLock()
-	defer conn.mu.RUnlock()
 	if conn.st != nil {
 		conn.cfg.Me = conn.st.Me()
 	}
@@ -230,6 +227,7 @@ func (conn *Conn) ConnectTo(host string, pass ...string) error {
 func (conn *Conn) Connect() error {
 	conn.mu.Lock()
 	defer conn.mu.Unlock()
+	conn.initialise()
 
 	if conn.cfg.Server == "" {
 		return fmt.Errorf("irc.Connect(): cfg.Server must be non-empty")
@@ -403,6 +401,9 @@ func (conn *Conn) rateLimit(chars int) time.Duration {
 }
 
 func (conn *Conn) shutdown() {
+	// Dispatch after closing connection but before reinit
+	// so event handlers can still access state information.
+	defer conn.dispatch(&Line{Cmd: DISCONNECTED, Time: time.Now()})
 	// Guard against double-call of shutdown() if we get an error in send()
 	// as calling sock.Close() will cause recv() to receive EOF in readstring()
 	conn.mu.Lock()
@@ -415,11 +416,6 @@ func (conn *Conn) shutdown() {
 	conn.sock.Close()
 	close(conn.die)
 	conn.wg.Wait()
-	// Dispatch after closing connection but before reinit
-	// so event handlers can still access state information.
-	conn.dispatch(&Line{Cmd: DISCONNECTED, Time: time.Now()})
-	// reinit datastructures ready for next connection
-	conn.initialise()
 }
 
 // Dumps a load of information about the current state of the connection to a
diff --git a/client/connection_test.go b/client/connection_test.go
index 3129c99..9b3bf63 100644
--- a/client/connection_test.go
+++ b/client/connection_test.go
@@ -51,6 +51,7 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
 	st := state.NewMockTracker(ctrl)
 	nc := MockNetConn(t)
 	c := SimpleClient("test", "test", "Testing IRC")
+	c.initialise()
 
 	c.st = st
 	c.sock = nc
@@ -66,7 +67,6 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
 }
 
 func (s *testState) tearDown() {
-	s.st.EXPECT().Wipe()
 	s.nc.ExpectNothing()
 	s.c.shutdown()
 	s.ctrl.Finish()
@@ -86,7 +86,6 @@ func TestEOF(t *testing.T) {
 	})
 
 	// Simulate EOF from server
-	s.st.EXPECT().Wipe()
 	s.nc.Close()
 
 	// Verify that disconnected handler was called
@@ -231,7 +230,6 @@ func TestSendExitsOnWriteError(t *testing.T) {
 	// Now, close the underlying socket to cause write() to return an error.
 	// This will call shutdown() => a call to st.Wipe() will happen.
 	exited.assertNotCalled("Exited before signal sent.")
-	s.st.EXPECT().Wipe()
 	s.nc.Close()
 	// Sending more on c.out shouldn't reach the network, but we need to send
 	// *something* to trigger a call to write() that will fail.
@@ -292,7 +290,6 @@ func TestRecv(t *testing.T) {
 
 	// The only way recv() exits is when the socket closes.
 	exited.assertNotCalled("Exited before socket close.")
-	s.st.EXPECT().Wipe()
 	s.nc.Close()
 	exited.assertWasCalled("Didn't exit on socket close.")