From e95c75f87d01f93019a6be1bbe8cc77645bf0f62 Mon Sep 17 00:00:00 2001
From: Daniel Oaks <daniel@danieloaks.net>
Date: Mon, 25 Sep 2017 11:29:27 +1000
Subject: [PATCH] monitor: Fix a crash around the MONITOR command

---
 irc/client.go  |  3 +++
 irc/monitor.go | 33 +++++++++++++++++++++++++++++++--
 irc/server.go  |  9 +++++----
 3 files changed, 39 insertions(+), 6 deletions(-)

diff --git a/irc/client.go b/irc/client.go
index 60762429..a9bb46a5 100644
--- a/irc/client.go
+++ b/irc/client.go
@@ -61,6 +61,7 @@ type Client struct {
 	isDestroyed        bool
 	isQuitting         bool
 	monitoring         map[string]bool
+	monitoringMutex    sync.RWMutex
 	nick               string
 	nickCasefolded     string
 	nickMaskCasefolded string
@@ -523,9 +524,11 @@ func (client *Client) destroy() {
 	}
 
 	// alert monitors
+	client.server.monitoringMutex.RLock()
 	for _, mClient := range client.server.monitoring[client.nickCasefolded] {
 		mClient.Send(nil, client.server.name, RPL_MONOFFLINE, mClient.nick, client.nick)
 	}
+	client.server.monitoringMutex.RUnlock()
 
 	// remove my monitors
 	client.clearMonitorList()
diff --git a/irc/monitor.go b/irc/monitor.go
index d7f750b8..a126ca6f 100644
--- a/irc/monitor.go
+++ b/irc/monitor.go
@@ -12,8 +12,13 @@ import (
 
 // alertMonitors alerts everyone monitoring us that we're online.
 func (client *Client) alertMonitors() {
+	// get monitors
+	client.server.monitoringMutex.RLock()
+	monitors := client.server.monitoring[client.nickCasefolded]
+	client.server.monitoringMutex.RUnlock()
+
 	// alert monitors
-	for _, mClient := range client.server.monitoring[client.nickCasefolded] {
+	for _, mClient := range monitors {
 		// don't have to notify ourselves
 		if mClient != client {
 			mClient.SendFromClient("", client, nil, RPL_MONONLINE, mClient.nick, client.nickMaskString)
@@ -23,6 +28,12 @@ func (client *Client) alertMonitors() {
 
 // clearMonitorList clears our MONITOR list.
 func (client *Client) clearMonitorList() {
+	// lockin' everything
+	client.monitoringMutex.Lock()
+	defer client.monitoringMutex.Unlock()
+	client.server.monitoringMutex.Lock()
+	defer client.server.monitoringMutex.Unlock()
+
 	for name := range client.monitoring {
 		// just removes current client from the list
 		orig := client.server.monitoring[name]
@@ -82,6 +93,9 @@ func monitorRemoveHandler(server *Server, client *Client, msg ircmsg.IrcMessage)
 			continue
 		}
 
+		client.monitoringMutex.Lock()
+		client.server.monitoringMutex.Lock()
+
 		if client.monitoring[casefoldedTarget] {
 			// just removes current client from the list
 			orig := server.monitoring[casefoldedTarget]
@@ -97,6 +111,9 @@ func monitorRemoveHandler(server *Server, client *Client, msg ircmsg.IrcMessage)
 			delete(client.monitoring, casefoldedTarget)
 		}
 
+		client.monitoringMutex.Unlock()
+		client.server.monitoringMutex.Unlock()
+
 		// remove first element of targets list
 		targets = targets[1:]
 	}
@@ -135,6 +152,9 @@ func monitorAddHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bo
 			continue
 		}
 
+		client.monitoringMutex.Lock()
+		client.server.monitoringMutex.Lock()
+
 		if !client.monitoring[casefoldedTarget] {
 			client.monitoring[casefoldedTarget] = true
 
@@ -142,6 +162,9 @@ func monitorAddHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bo
 			server.monitoring[casefoldedTarget] = append(orig, client)
 		}
 
+		client.monitoringMutex.Unlock()
+		client.server.monitoringMutex.Unlock()
+
 		// add to online / offline lists
 		target := server.clients.Get(casefoldedTarget)
 		if target == nil {
@@ -172,9 +195,11 @@ func monitorClearHandler(server *Server, client *Client, msg ircmsg.IrcMessage)
 
 func monitorListHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 	var monitorList []string
+	client.monitoringMutex.RLock()
 	for name := range client.monitoring {
 		monitorList = append(monitorList, name)
 	}
+	client.monitoringMutex.RUnlock()
 
 	for _, line := range argsToStrings(maxLastArgLength, monitorList, ",") {
 		client.Send(nil, server.name, RPL_MONLIST, client.nick, line)
@@ -187,7 +212,11 @@ func monitorStatusHandler(server *Server, client *Client, msg ircmsg.IrcMessage)
 	var online []string
 	var offline []string
 
-	for name := range client.monitoring {
+	client.monitoringMutex.RLock()
+	monitoring := client.monitoring
+	client.monitoringMutex.RUnlock()
+
+	for name := range monitoring {
 		target := server.clients.Get(name)
 		if target == nil {
 			offline = append(offline, name)
diff --git a/irc/server.go b/irc/server.go
index 2a5f9e90..d908aa74 100644
--- a/irc/server.go
+++ b/irc/server.go
@@ -59,11 +59,11 @@ type LineLenLimits struct {
 
 // ListenerWrapper wraps a listener so it can be safely reconfigured or stopped
 type ListenerWrapper struct {
-	listener    net.Listener
-	tlsConfig   *tls.Config
-	shouldStop  bool
+	listener   net.Listener
+	tlsConfig  *tls.Config
+	shouldStop bool
 	// lets the ListenerWrapper inform the server that it has stopped:
-	stopEvent   chan bool
+	stopEvent chan bool
 	// protects atomic update of tlsConfig and shouldStop:
 	configMutex sync.Mutex
 }
@@ -95,6 +95,7 @@ type Server struct {
 	logger                       *logger.Manager
 	MaxSendQBytes                uint64
 	monitoring                   map[string][]*Client
+	monitoringMutex              sync.RWMutex
 	motdLines                    []string
 	name                         string
 	nameCasefolded               string