Changed the way Quit/Disconnect works to avoid possible DATA races.

Rewrote tests to be less spammy. Created REconnection test.

irc.go

@@ -183,13 +183,20 @@ func (irc *Connection) pingLoop() {
 	}
 }
 
+func (irc *Connection) isQuitting() bool {
+	irc.Lock()
+	defer irc.Unlock()
+	return irc.quit
+}
+
 // Main loop to control the connection.
 func (irc *Connection) Loop() {
 	errChan := irc.ErrorChan()
-	for !irc.quit {
+	for !irc.isQuitting() {
 		err := <-errChan
-		irc.Log.Printf("Error, disconnected: %s\n", err)
-		for !irc.quit {
+		irc.Wait()
+		for !irc.isQuitting() {
+			irc.Log.Printf("Error, disconnected: %s\n", err)
 			if err = irc.Reconnect(); err != nil {
 				irc.Log.Printf("Error while reconnecting: %s\n", err)
 				time.Sleep(60 * time.Second)
@@ -211,8 +218,10 @@ func (irc *Connection) Quit() {
 	}
 
 	irc.SendRaw(quit)
+	irc.Lock()
 	irc.stopped = true
 	irc.quit = true
+	irc.Unlock()
 }
 
 // Use the connection to join a given channel.
@@ -341,37 +350,15 @@ func (irc *Connection) Connected() bool {
 // A disconnect sends all buffered messages (if possible),
 // stops all goroutines and then closes the socket.
 func (irc *Connection) Disconnect() {
-	for event := range irc.events {
-		irc.ClearCallback(event)
-	}
-	if irc.end != nil {
-		close(irc.end)
-	}
-
-	irc.end = nil
-
-	if irc.pwrite != nil {
-		close(irc.pwrite)
-	}
-
-	irc.Wait()
 	if irc.socket != nil {
 		irc.socket.Close()
 	}
-	irc.socket = nil
+	close(irc.end)
 	irc.ErrorChan() <- ErrDisconnected
 }
 
 // Reconnect to a server using the current connection.
 func (irc *Connection) Reconnect() error {
-	if irc.end != nil {
-		close(irc.end)
-	}
-
-	irc.end = nil
-
-	irc.Wait() //make sure that wait group is cleared ensuring that all spawned goroutines have completed
-
 	irc.end = make(chan struct{})
 	return irc.Connect(irc.Server)
 }

irc_callback.go

@@ -138,7 +138,7 @@ func (irc *Connection) setupCallbacks() {
 
 	//Handle error events. This has to be called in a new thred to allow
 	//readLoop to exit
-	irc.AddCallback("ERROR", func(e *Event) { go irc.Disconnect() })
+	irc.AddCallback("ERROR", func(e *Event) { irc.Disconnect() })
 
 	//Handle ping events
 	irc.AddCallback("PING", func(e *Event) { irc.SendRaw("PONG :" + e.Message()) })

irc_struct.go

@@ -13,6 +13,7 @@ import (
 )
 
 type Connection struct {
+	sync.Mutex
 	sync.WaitGroup
 	Debug        bool
 	Error        chan error
@@ -46,7 +47,7 @@ type Connection struct {
 	Log                    *log.Logger
 
 	stopped bool
-	quit    bool
+	quit    bool //User called Quit, do not reconnect.
 }
 
 // A struct to represent an event.

irc_test.go

@@ -12,6 +12,10 @@ const serverssl = "irc.freenode.net:7000"
 const channel = "#go-eventirc-test"
 const dict = "abcdefghijklmnopqrstuvwxyz"
 
+//Spammy
+const verbose_tests = false
+const debug_tests = true
+
 func TestConnectionEmtpyServer(t *testing.T) {
 	irccon := IRC("go-eventirc", "go-eventirc")
 	err := irccon.Connect("")
@@ -91,8 +95,8 @@ func TestConnectionEmptyNick(t *testing.T) {
 
 func TestRemoveCallback(t *testing.T) {
 	irccon := IRC("go-eventirc", "go-eventirc")
-	irccon.VerboseCallbackHandler = true
-	irccon.Debug = true
+	irccon.VerboseCallbackHandler = verbose_tests
+	irccon.Debug = debug_tests
 
 	done := make(chan int, 10)
 
@@ -119,8 +123,8 @@ func TestRemoveCallback(t *testing.T) {
 
 func TestWildcardCallback(t *testing.T) {
 	irccon := IRC("go-eventirc", "go-eventirc")
-	irccon.VerboseCallbackHandler = true
-	irccon.Debug = true
+	irccon.VerboseCallbackHandler = verbose_tests
+	irccon.Debug = debug_tests
 
 	done := make(chan int, 10)
 
@@ -143,8 +147,8 @@ func TestWildcardCallback(t *testing.T) {
 
 func TestClearCallback(t *testing.T) {
 	irccon := IRC("go-eventirc", "go-eventirc")
-	irccon.VerboseCallbackHandler = true
-	irccon.Debug = true
+	irccon.VerboseCallbackHandler = verbose_tests
+	irccon.Debug = debug_tests
 
 	done := make(chan int, 10)
 
@@ -188,14 +192,14 @@ func TestConnection(t *testing.T) {
 	ircnick1 := randStr(8)
 	ircnick2 := randStr(8)
 	irccon1 := IRC(ircnick1, "IRCTest1")
-	irccon1.VerboseCallbackHandler = true
-	irccon1.Debug = true
+	irccon1.VerboseCallbackHandler = verbose_tests
+	irccon1.Debug = debug_tests
 	irccon2 := IRC(ircnick2, "IRCTest2")
-	irccon2.VerboseCallbackHandler = true
-	irccon2.Debug = true
+	irccon2.VerboseCallbackHandler = verbose_tests
+	irccon2.Debug = debug_tests
 
 	teststr := randStr(20)
-	testmsgok := false
+	testmsgok := make(chan bool, 1)
 
 	irccon1.AddCallback("001", func(e *Event) { irccon1.Join(channel) })
 	irccon2.AddCallback("001", func(e *Event) { irccon2.Join(channel) })
@@ -204,13 +208,18 @@ func TestConnection(t *testing.T) {
 			tick := time.NewTicker(1 * time.Second)
 			i := 10
 			for {
-				<-tick.C
-				irccon1.Privmsgf(channel, "%s\n", teststr)
-				if testmsgok {
+				select {
+				case <-tick.C:
+					irccon1.Privmsgf(channel, "%s\n", teststr)
+					if i == 0 {
+						t.Errorf("Timeout while wating for test message from the other thread.")
+						return
+					}
+
+				case <-testmsgok:
 					tick.Stop()
 					irccon1.Quit()
-				} else if i == 0 {
-					t.Fatal("Timeout while wating for test message from the other thread.")
+					return
 				}
 				i -= 1
 			}
@@ -223,43 +232,77 @@ func TestConnection(t *testing.T) {
 	})
 
 	irccon2.AddCallback("PRIVMSG", func(e *Event) {
-		t.Log(e.Message())
 		if e.Message() == teststr {
 			if e.Nick == ircnick1 {
-				testmsgok = true
+				testmsgok <- true
 				irccon2.Quit()
 			} else {
-				t.Fatal("Test message came from an unexpected nickname")
+				t.Errorf("Test message came from an unexpected nickname")
 			}
+		} else {
+			//this may fail if there are other incoming messages, unlikely.
+			t.Errorf("Test message mismatch")
 		}
 	})
 
 	irccon2.AddCallback("NICK", func(e *Event) {
 		if irccon2.nickcurrent == ircnick2 {
-			t.Fatal("Nick change did not work!")
+			t.Errorf("Nick change did not work!")
 		}
 	})
 
 	err := irccon1.Connect(server)
 	if err != nil {
 		t.Log(err.Error())
-		t.Fatal("Can't connect to freenode.")
+		t.Errorf("Can't connect to freenode.")
 	}
 	err = irccon2.Connect(server)
 	if err != nil {
 		t.Log(err.Error())
-		t.Fatal("Can't connect to freenode.")
+		t.Errorf("Can't connect to freenode.")
 	}
 
 	go irccon2.Loop()
 	irccon1.Loop()
 }
 
+func TestReconnect(t *testing.T) {
+	ircnick1 := randStr(8)
+	irccon := IRC(ircnick1, "IRCTestRe")
+	irccon.VerboseCallbackHandler = verbose_tests
+	irccon.Debug = debug_tests
+	connects := 0
+	irccon.AddCallback("001", func(e *Event) { irccon.Join(channel) })
+
+	irccon.AddCallback("366", func(e *Event) {
+		connects += 1
+		if connects > 2 {
+			irccon.Privmsgf(channel, "Connection nr %d (test done)\n", connects)
+			irccon.Quit()
+		} else {
+			irccon.Privmsgf(channel, "Connection nr %d\n", connects)
+			time.Sleep(100) //Meed to let the thraed actually send before closing socket
+			irccon.Disconnect()
+		}
+	})
+
+	err := irccon.Connect(server)
+	if err != nil {
+		t.Log(err.Error())
+		t.Errorf("Can't connect to freenode.")
+	}
+
+	irccon.Loop()
+	if connects != 3 {
+		t.Errorf("Reconnect test failed. Connects = %d", connects)
+	}
+}
+
 func TestConnectionSSL(t *testing.T) {
 	ircnick1 := randStr(8)
 	irccon := IRC(ircnick1, "IRCTestSSL")
-	irccon.VerboseCallbackHandler = true
-	irccon.Debug = true
+	irccon.VerboseCallbackHandler = verbose_tests
+	irccon.Debug = debug_tests
 	irccon.UseTLS = true
 	irccon.TLSConfig = &tls.Config{InsecureSkipVerify: true}
 	irccon.AddCallback("001", func(e *Event) { irccon.Join(channel) })
@@ -272,7 +315,7 @@ func TestConnectionSSL(t *testing.T) {
 	err := irccon.Connect(serverssl)
 	if err != nil {
 		t.Log(err.Error())
-		t.Fatal("Can't connect to freenode.")
+		t.Errorf("Can't connect to freenode.")
 	}
 
 	irccon.Loop()