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.
This commit is contained in:
Alex Bramley 2015-04-13 19:05:42 +01:00
parent e1ddd58df4
commit cd24432da4
2 changed files with 5 additions and 12 deletions

View File

@ -154,7 +154,6 @@ func Client(cfg *Config) *Conn {
lastsent: time.Now(), lastsent: time.Now(),
} }
conn.addIntHandlers() conn.addIntHandlers()
conn.initialise()
return conn return conn
} }
@ -169,8 +168,6 @@ func (conn *Conn) Config() *Config {
} }
func (conn *Conn) Me() *state.Nick { func (conn *Conn) Me() *state.Nick {
conn.mu.RLock()
defer conn.mu.RUnlock()
if conn.st != nil { if conn.st != nil {
conn.cfg.Me = conn.st.Me() conn.cfg.Me = conn.st.Me()
} }
@ -230,6 +227,7 @@ func (conn *Conn) ConnectTo(host string, pass ...string) error {
func (conn *Conn) Connect() error { func (conn *Conn) Connect() error {
conn.mu.Lock() conn.mu.Lock()
defer conn.mu.Unlock() defer conn.mu.Unlock()
conn.initialise()
if conn.cfg.Server == "" { if conn.cfg.Server == "" {
return fmt.Errorf("irc.Connect(): cfg.Server must be non-empty") 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() { 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() // 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() // as calling sock.Close() will cause recv() to receive EOF in readstring()
conn.mu.Lock() conn.mu.Lock()
@ -415,11 +416,6 @@ func (conn *Conn) shutdown() {
conn.sock.Close() conn.sock.Close()
close(conn.die) close(conn.die)
conn.wg.Wait() 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 // Dumps a load of information about the current state of the connection to a

View File

@ -51,6 +51,7 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
st := state.NewMockTracker(ctrl) st := state.NewMockTracker(ctrl)
nc := MockNetConn(t) nc := MockNetConn(t)
c := SimpleClient("test", "test", "Testing IRC") c := SimpleClient("test", "test", "Testing IRC")
c.initialise()
c.st = st c.st = st
c.sock = nc c.sock = nc
@ -66,7 +67,6 @@ func setUp(t *testing.T, start ...bool) (*Conn, *testState) {
} }
func (s *testState) tearDown() { func (s *testState) tearDown() {
s.st.EXPECT().Wipe()
s.nc.ExpectNothing() s.nc.ExpectNothing()
s.c.shutdown() s.c.shutdown()
s.ctrl.Finish() s.ctrl.Finish()
@ -86,7 +86,6 @@ func TestEOF(t *testing.T) {
}) })
// Simulate EOF from server // Simulate EOF from server
s.st.EXPECT().Wipe()
s.nc.Close() s.nc.Close()
// Verify that disconnected handler was called // 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. // 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 shutdown() => a call to st.Wipe() will happen.
exited.assertNotCalled("Exited before signal sent.") exited.assertNotCalled("Exited before signal sent.")
s.st.EXPECT().Wipe()
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
// *something* to trigger a call to write() that will fail. // *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. // The only way recv() exits is when the socket closes.
exited.assertNotCalled("Exited before socket close.") exited.assertNotCalled("Exited before socket close.")
s.st.EXPECT().Wipe()
s.nc.Close() s.nc.Close()
exited.assertWasCalled("Didn't exit on socket close.") exited.assertWasCalled("Didn't exit on socket close.")