Squash a bunch of possible races

irc/client.go

@@ -13,6 +13,7 @@ import (
 	"runtime/debug"
 	"strconv"
 	"strings"
+	"sync"
 	"time"
 
 	"github.com/DanielOaks/girc-go/ircmsg"
@@ -43,23 +44,24 @@ type Client struct {
 	channels           ChannelSet
 	class              *OperClass
 	ctime              time.Time
+	destroyMutex       sync.Mutex
 	flags              map[Mode]bool
-	isDestroyed        bool
-	isQuitting         bool
 	hasQuit            bool
 	hops               int
 	hostname           string
-	rawHostname        string
-	vhost              string
 	idleTimer          *time.Timer
+	isDestroyed        bool
+	isQuitting         bool
 	monitoring         map[string]bool
 	nick               string
 	nickCasefolded     string
-	nickMaskString     string // cache for nickmask string since it's used with lots of replies
 	nickMaskCasefolded string
+	nickMaskString     string // cache for nickmask string since it's used with lots of replies
 	operName           string
-	quitTimer          *time.Timer
 	quitMessageSent    bool
+	quitMutex          sync.Mutex
+	quitTimer          *time.Timer
+	rawHostname        string
 	realname           string
 	registered         bool
 	saslInProgress     bool
@@ -67,7 +69,9 @@ type Client struct {
 	saslValue          string
 	server             *Server
 	socket             *Socket
+	timerMutex         sync.Mutex
 	username           string
+	vhost              string
 	whoisLine          string
 }
 
@@ -229,6 +233,9 @@ func (client *Client) Active() {
 
 // Touch marks the client as alive.
 func (client *Client) Touch() {
+	client.timerMutex.Lock()
+	defer client.timerMutex.Unlock()
+
 	if client.quitTimer != nil {
 		client.quitTimer.Stop()
 	}
@@ -242,6 +249,9 @@ func (client *Client) Touch() {
 
 // Idle resets the timeout handlers and sends the client a PING.
 func (client *Client) Idle() {
+	client.timerMutex.Lock()
+	defer client.timerMutex.Unlock()
+
 	client.Send(nil, "", "PING", client.nick)
 
 	if client.quitTimer == nil {
@@ -435,6 +445,8 @@ func (client *Client) ChangeNickname(nickname string) error {
 
 // Quit sends the given quit message to the client (but does not destroy them).
 func (client *Client) Quit(message string) {
+	client.quitMutex.Lock()
+	defer client.quitMutex.Unlock()
 	if !client.quitMessageSent {
 		quitMsg := ircmsg.MakeMessage(nil, client.nickMaskString, "QUIT", message)
 		quitLine, _ := quitMsg.Line()
@@ -442,13 +454,15 @@ func (client *Client) Quit(message string) {
 		errorMsg := ircmsg.MakeMessage(nil, "", "ERROR", message)
 		errorLine, _ := errorMsg.Line()
 
-		client.socket.FinalData = quitLine + errorLine
+		client.socket.SetFinalData(quitLine + errorLine)
 		client.quitMessageSent = true
 	}
 }
 
 // destroy gets rid of a client, removes them from server lists etc.
 func (client *Client) destroy() {
+	client.destroyMutex.Lock()
+	defer client.destroyMutex.Unlock()
 	if client.isDestroyed {
 		return
 	}

irc/socket.go

@@ -10,7 +10,6 @@ import (
 	"crypto/tls"
 	"encoding/hex"
 	"errors"
-	"fmt"
 	"io"
 	"net"
 	"strings"
@@ -26,12 +25,16 @@ var (
 
 // Socket represents an IRC socket.
 type Socket struct {
-	Closed bool
 	conn   net.Conn
 	reader *bufio.Reader
 
 	MaxSendQBytes uint64
-	FinalData     string // what to send when we die
+
+	closed      bool
+	closedMutex sync.Mutex
+
+	finalData      string // what to send when we die
+	finalDataMutex sync.Mutex
 
 	lineToSendExists chan bool
 	linesToSend      []string
@@ -50,10 +53,12 @@ func NewSocket(conn net.Conn, maxSendQBytes uint64) Socket {
 
 // Close stops a Socket from being able to send/receive any more data.
 func (socket *Socket) Close() {
-	if socket.Closed {
+	socket.closedMutex.Lock()
+	defer socket.closedMutex.Unlock()
+	if socket.closed {
 		return
 	}
-	socket.Closed = true
+	socket.closed = true
 
 	// force close loop to happen if it hasn't already
 	go socket.timedFillLineToSendExists(200 * time.Millisecond)
@@ -88,7 +93,7 @@ func (socket *Socket) CertFP() (string, error) {
 
 // Read returns a single IRC line from a Socket.
 func (socket *Socket) Read() (string, error) {
-	if socket.Closed {
+	if socket.IsClosed() {
 		return "", io.EOF
 	}
 
@@ -113,7 +118,7 @@ func (socket *Socket) Read() (string, error) {
 
 // Write sends the given string out of Socket.
 func (socket *Socket) Write(data string) error {
-	if socket.Closed {
+	if socket.IsClosed() {
 		return io.EOF
 	}
 
@@ -121,9 +126,7 @@ func (socket *Socket) Write(data string) error {
 	socket.linesToSend = append(socket.linesToSend, data)
 	socket.linesToSendMutex.Unlock()
 
-	if !socket.Closed {
+	go socket.timedFillLineToSendExists(15 * time.Second)
-		go socket.timedFillLineToSendExists(15 * time.Second)
-	}
 
 	return nil
 }
@@ -138,9 +141,22 @@ func (socket *Socket) timedFillLineToSendExists(duration time.Duration) {
 	}
 }
 
+// SetFinalData sets the final data to send when the SocketWriter closes.
+func (socket *Socket) SetFinalData(data string) {
+	socket.finalDataMutex.Lock()
+	socket.finalData = data
+	socket.finalDataMutex.Unlock()
+}
+
+// IsClosed returns whether the socket is closed.
+func (socket *Socket) IsClosed() bool {
+	socket.closedMutex.Lock()
+	defer socket.closedMutex.Unlock()
+	return socket.closed
+}
+
 // RunSocketWriter starts writing messages to the outgoing socket.
 func (socket *Socket) RunSocketWriter() {
-	var errOut bool
 	for {
 		// wait for new lines
 		select {
@@ -148,7 +164,7 @@ func (socket *Socket) RunSocketWriter() {
 			socket.linesToSendMutex.Lock()
 
 			// check if we're closed
-			if socket.Closed {
+			if socket.IsClosed() {
 				socket.linesToSendMutex.Unlock()
 				break
 			}
@@ -169,7 +185,7 @@ func (socket *Socket) RunSocketWriter() {
 				}
 			}
 			if socket.MaxSendQBytes < sendQBytes {
-				socket.FinalData = "\r\nERROR :SendQ Exceeded\r\n"
+				socket.SetFinalData("\r\nERROR :SendQ Exceeded\r\n")
 				socket.linesToSendMutex.Unlock()
 				break
 			}
@@ -184,24 +200,30 @@ func (socket *Socket) RunSocketWriter() {
 			if 0 < len(data) {
 				_, err := socket.conn.Write([]byte(data))
 				if err != nil {
-					errOut = true
-					fmt.Println(err.Error())
 					break
 				}
 			}
 		}
-		if errOut || socket.Closed {
+		if socket.IsClosed() {
 			// error out or we've been closed
 			break
 		}
 	}
-	if !socket.Closed {
+	// force closure of socket
-		socket.Closed = true
+	socket.closedMutex.Lock()
+	if !socket.closed {
+		socket.closed = true
 	}
+	socket.closedMutex.Unlock()
+
 	// write error lines
-	if 0 < len(socket.FinalData) {
+	socket.finalDataMutex.Lock()
-		socket.conn.Write([]byte(socket.FinalData))
+	if 0 < len(socket.finalData) {
+		socket.conn.Write([]byte(socket.finalData))
 	}
+	socket.finalDataMutex.Unlock()
+
+	// close the connection
 	socket.conn.Close()
 
 	// empty the lineToSendExists channel

irc/whowas.go

@@ -4,10 +4,16 @@
 
 package irc
 
+import (
+	"sync"
+)
+
 type WhoWasList struct {
 	buffer []*WhoWas
 	start  int
 	end    int
+
+	accessMutex sync.RWMutex
 }
 
 type WhoWas struct {
@@ -25,6 +31,9 @@ func NewWhoWasList(size uint) *WhoWasList {
 }
 
 func (list *WhoWasList) Append(client *Client) {
+	list.accessMutex.Lock()
+	defer list.accessMutex.Unlock()
+
 	list.buffer[list.end] = &WhoWas{
 		nicknameCasefolded: client.nickCasefolded,
 		nickname:           client.nick,
@@ -39,6 +48,9 @@ func (list *WhoWasList) Append(client *Client) {
 }
 
 func (list *WhoWasList) Find(nickname string, limit int64) []*WhoWas {
+	list.accessMutex.RLock()
+	defer list.accessMutex.RUnlock()
+
 	results := make([]*WhoWas, 0)
 
 	casefoldedNickname, err := CasefoldName(nickname)
@@ -59,6 +71,9 @@ func (list *WhoWasList) Find(nickname string, limit int64) []*WhoWas {
 }
 
 func (list *WhoWasList) prev(index int) int {
+	list.accessMutex.RLock()
+	defer list.accessMutex.RUnlock()
+
 	index--
 	if index < 0 {
 		index += len(list.buffer)