From c911ff2bcdc0c89fff076dbdf550ac456470f224 Mon Sep 17 00:00:00 2001 From: Daniel Oaks Date: Tue, 18 Apr 2017 22:26:01 +1000 Subject: [PATCH] Squash a bunch of possible races --- irc/client.go | 28 +++++++++++++++++------ irc/socket.go | 62 ++++++++++++++++++++++++++++++++++----------------- irc/whowas.go | 15 +++++++++++++ 3 files changed, 78 insertions(+), 27 deletions(-) diff --git a/irc/client.go b/irc/client.go index 55ad339e..848d25a2 100644 --- a/irc/client.go +++ b/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 } diff --git a/irc/socket.go b/irc/socket.go index d7e7b4b6..7e26727d 100644 --- a/irc/socket.go +++ b/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 { - socket.Closed = true + // force closure of socket + socket.closedMutex.Lock() + if !socket.closed { + socket.closed = true } + socket.closedMutex.Unlock() + // write error lines - if 0 < len(socket.FinalData) { - socket.conn.Write([]byte(socket.FinalData)) + socket.finalDataMutex.Lock() + if 0 < len(socket.finalData) { + socket.conn.Write([]byte(socket.finalData)) } + socket.finalDataMutex.Unlock() + + // close the connection socket.conn.Close() // empty the lineToSendExists channel diff --git a/irc/whowas.go b/irc/whowas.go index 14aedec5..f5ebbe8f 100644 --- a/irc/whowas.go +++ b/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)