Merge pull request #146 from slingamn/data_races.1

fix various data races, including 2 introduced by #139

irc/client.go

@@ -94,7 +94,7 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client {
 	go socket.RunSocketWriter()
 	client := &Client{
 		atime:          now,
-		authorized:     server.password == nil,
+		authorized:     server.getPassword() == nil,
 		capabilities:   caps.NewSet(),
 		capState:       CapNone,
 		capVersion:     caps.Cap301,
@@ -182,10 +182,11 @@ func (client *Client) maxlens() (int, int) {
 		maxlenTags = 4096
 	}
 	if client.capabilities.Has(caps.MaxLine) {
-		if client.server.limits.LineLen.Tags > maxlenTags {
-			maxlenTags = client.server.limits.LineLen.Tags
+		limits := client.server.getLimits()
+		if limits.LineLen.Tags > maxlenTags {
+			maxlenTags = limits.LineLen.Tags
 		}
-		maxlenRest = client.server.limits.LineLen.Rest
+		maxlenRest = limits.LineLen.Rest
 	}
 	return maxlenTags, maxlenRest
 }
@@ -679,7 +680,7 @@ func (client *Client) Send(tags *map[string]ircmsg.TagValue, prefix string, comm
 func (client *Client) Notice(text string) {
 	limit := 400
 	if client.capabilities.Has(caps.MaxLine) {
-		limit = client.server.limits.LineLen.Rest - 110
+		limit = client.server.getLimits().LineLen.Rest - 110
 	}
 	lines := wordWrap(text, limit)
 

irc/getters.go

@@ -0,0 +1,22 @@
+// Copyright (c) 2017 Shivaram Lingamneni <slingamn@cs.stanford.edu>
+// released under the MIT license
+
+package irc
+
+func (server *Server) getISupport() *ISupportList {
+	server.configurableStateMutex.RLock()
+	defer server.configurableStateMutex.RUnlock()
+	return server.isupport
+}
+
+func (server *Server) getLimits() Limits {
+	server.configurableStateMutex.RLock()
+	defer server.configurableStateMutex.RUnlock()
+	return server.limits
+}
+
+func (server *Server) getPassword() []byte {
+	server.configurableStateMutex.RLock()
+	defer server.configurableStateMutex.RUnlock()
+	return server.password
+}

irc/isupport.go

@@ -142,7 +142,7 @@ func (il *ISupportList) RegenerateCachedReply() {
 
 // RplISupport outputs our ISUPPORT lines to the client. This is used on connection and in VERSION responses.
 func (client *Client) RplISupport() {
-	for _, tokenline := range client.server.isupport.CachedReply {
+	for _, tokenline := range client.server.getISupport().CachedReply {
 		// ugly trickery ahead
 		client.Send(nil, client.server.name, RPL_ISUPPORT, append([]string{client.nick}, tokenline...)...)
 	}

irc/nickname.go

@@ -34,7 +34,7 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 		return false
 	}
 
-	if err != nil || len(nicknameRaw) > server.limits.NickLen || restrictedNicknames[nickname] {
+	if err != nil || len(nicknameRaw) > server.getLimits().NickLen || restrictedNicknames[nickname] {
 		client.Send(nil, server.name, ERR_ERRONEUSNICKNAME, client.nick, nicknameRaw, "Erroneous nickname")
 		return false
 	}

irc/server.go

@@ -182,29 +182,31 @@ func NewServer(config *Config, logger *logger.Manager) (*Server, error) {
 func (server *Server) setISupport() {
 	maxTargetsString := strconv.Itoa(maxTargets)
 
+	server.configurableStateMutex.RLock()
+
 	// add RPL_ISUPPORT tokens
-	server.isupport = NewISupportList()
-	server.isupport.Add("AWAYLEN", strconv.Itoa(server.limits.AwayLen))
-	server.isupport.Add("CASEMAPPING", casemappingName)
-	server.isupport.Add("CHANMODES", strings.Join([]string{Modes{BanMask, ExceptMask, InviteMask}.String(), "", Modes{UserLimit, Key}.String(), Modes{InviteOnly, Moderated, NoOutside, OpOnlyTopic, ChanRoleplaying, Secret}.String()}, ","))
-	server.isupport.Add("CHANNELLEN", strconv.Itoa(server.limits.ChannelLen))
-	server.isupport.Add("CHANTYPES", "#")
-	server.isupport.Add("ELIST", "U")
-	server.isupport.Add("EXCEPTS", "")
-	server.isupport.Add("INVEX", "")
-	server.isupport.Add("KICKLEN", strconv.Itoa(server.limits.KickLen))
-	server.isupport.Add("MAXLIST", fmt.Sprintf("beI:%s", strconv.Itoa(server.limits.ChanListModes)))
-	server.isupport.Add("MAXTARGETS", maxTargetsString)
-	server.isupport.Add("MODES", "")
-	server.isupport.Add("MONITOR", strconv.Itoa(server.limits.MonitorEntries))
-	server.isupport.Add("NETWORK", server.networkName)
-	server.isupport.Add("NICKLEN", strconv.Itoa(server.limits.NickLen))
-	server.isupport.Add("PREFIX", "(qaohv)~&@%+")
-	server.isupport.Add("RPCHAN", "E")
-	server.isupport.Add("RPUSER", "E")
-	server.isupport.Add("STATUSMSG", "~&@%+")
-	server.isupport.Add("TARGMAX", fmt.Sprintf("NAMES:1,LIST:1,KICK:1,WHOIS:1,USERHOST:10,PRIVMSG:%s,TAGMSG:%s,NOTICE:%s,MONITOR:", maxTargetsString, maxTargetsString, maxTargetsString))
-	server.isupport.Add("TOPICLEN", strconv.Itoa(server.limits.TopicLen))
+	isupport := NewISupportList()
+	isupport.Add("AWAYLEN", strconv.Itoa(server.limits.AwayLen))
+	isupport.Add("CASEMAPPING", casemappingName)
+	isupport.Add("CHANMODES", strings.Join([]string{Modes{BanMask, ExceptMask, InviteMask}.String(), "", Modes{UserLimit, Key}.String(), Modes{InviteOnly, Moderated, NoOutside, OpOnlyTopic, ChanRoleplaying, Secret}.String()}, ","))
+	isupport.Add("CHANNELLEN", strconv.Itoa(server.limits.ChannelLen))
+	isupport.Add("CHANTYPES", "#")
+	isupport.Add("ELIST", "U")
+	isupport.Add("EXCEPTS", "")
+	isupport.Add("INVEX", "")
+	isupport.Add("KICKLEN", strconv.Itoa(server.limits.KickLen))
+	isupport.Add("MAXLIST", fmt.Sprintf("beI:%s", strconv.Itoa(server.limits.ChanListModes)))
+	isupport.Add("MAXTARGETS", maxTargetsString)
+	isupport.Add("MODES", "")
+	isupport.Add("MONITOR", strconv.Itoa(server.limits.MonitorEntries))
+	isupport.Add("NETWORK", server.networkName)
+	isupport.Add("NICKLEN", strconv.Itoa(server.limits.NickLen))
+	isupport.Add("PREFIX", "(qaohv)~&@%+")
+	isupport.Add("RPCHAN", "E")
+	isupport.Add("RPUSER", "E")
+	isupport.Add("STATUSMSG", "~&@%+")
+	isupport.Add("TARGMAX", fmt.Sprintf("NAMES:1,LIST:1,KICK:1,WHOIS:1,USERHOST:10,PRIVMSG:%s,TAGMSG:%s,NOTICE:%s,MONITOR:", maxTargetsString, maxTargetsString, maxTargetsString))
+	isupport.Add("TOPICLEN", strconv.Itoa(server.limits.TopicLen))
 
 	// account registration
 	if server.accountRegistration.Enabled {
@@ -216,12 +218,18 @@ func (server *Server) setISupport() {
 			}
 		}
 
-		server.isupport.Add("REGCOMMANDS", "CREATE,VERIFY")
-		server.isupport.Add("REGCALLBACKS", strings.Join(enabledCallbacks, ","))
-		server.isupport.Add("REGCREDTYPES", "passphrase,certfp")
+		isupport.Add("REGCOMMANDS", "CREATE,VERIFY")
+		isupport.Add("REGCALLBACKS", strings.Join(enabledCallbacks, ","))
+		isupport.Add("REGCREDTYPES", "passphrase,certfp")
 	}
 
-	server.isupport.RegenerateCachedReply()
+	server.configurableStateMutex.RUnlock()
+
+	isupport.RegenerateCachedReply()
+
+	server.configurableStateMutex.Lock()
+	server.isupport = isupport
+	server.configurableStateMutex.Unlock()
 }
 
 func loadChannelList(channel *Channel, list string, maskMode Mode) {
@@ -440,15 +448,16 @@ func (server *Server) tryRegister(c *Client) {
 // MOTD serves the Message of the Day.
 func (server *Server) MOTD(client *Client) {
 	server.configurableStateMutex.RLock()
-	defer server.configurableStateMutex.RUnlock()
+	motdLines := server.motdLines
+	server.configurableStateMutex.RUnlock()
 
-	if len(server.motdLines) < 1 {
+	if len(motdLines) < 1 {
 		client.Send(nil, server.name, ERR_NOMOTD, client.nick, "MOTD File is missing")
 		return
 	}
 
 	client.Send(nil, server.name, RPL_MOTDSTART, client.nick, fmt.Sprintf("- %s Message of the day - ", server.name))
-	for _, line := range server.motdLines {
+	for _, line := range motdLines {
 		client.Send(nil, server.name, RPL_MOTD, client.nick, line)
 	}
 	client.Send(nil, server.name, RPL_ENDOFMOTD, client.nick, "End of MOTD command")
@@ -691,7 +700,7 @@ func joinHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 
 		channel := server.channels.Get(casefoldedName)
 		if channel == nil {
-			if len(casefoldedName) > server.limits.ChannelLen {
+			if len(casefoldedName) > server.getLimits().ChannelLen {
 				client.Send(nil, server.name, ERR_NOSUCHCHANNEL, client.nick, name, "No such channel")
 				continue
 			}
@@ -1257,15 +1266,19 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
 
 	// sanity checks complete, start modifying server state
 
-	server.name = config.Server.Name
-	server.nameCasefolded = casefoldedName
+	if initial {
+		server.name = config.Server.Name
+		server.nameCasefolded = casefoldedName
+	}
 	server.networkName = config.Network.Name
 
+	server.configurableStateMutex.Lock()
 	if config.Server.Password != "" {
 		server.password = config.Server.PasswordBytes()
 	} else {
 		server.password = nil
 	}
+	server.configurableStateMutex.Unlock()
 
 	// apply new PROXY command restrictions
 	server.proxyAllowedFrom = config.Server.ProxyAllowedFrom
@@ -1372,6 +1385,7 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
 	}
 
 	// set server options
+	server.configurableStateMutex.Lock()
 	lineLenConfig := LineLenLimits{
 		Tags: config.Limits.LineLen.Tags,
 		Rest: config.Limits.LineLen.Rest,
@@ -1395,13 +1409,14 @@ func (server *Server) applyConfig(config *Config, initial bool) error {
 	server.accountRegistration = &accountReg
 	server.channelRegistrationEnabled = config.Channels.Registration.Enabled
 
-	server.configurableStateMutex.Lock()
 	server.defaultChannelModes = ParseDefaultChannelModes(config)
 	server.configurableStateMutex.Unlock()
 
 	// set new sendqueue size
 	if config.Server.MaxSendQBytes != server.MaxSendQBytes {
+		server.configurableStateMutex.Lock()
 		server.MaxSendQBytes = config.Server.MaxSendQBytes
+		server.configurableStateMutex.Unlock()
 
 		// update on all clients
 		server.clients.ByNickMutex.RLock()
@@ -1469,8 +1484,8 @@ func (server *Server) loadMOTD(motdPath string) error {
 	}
 
 	server.configurableStateMutex.Lock()
-	defer server.configurableStateMutex.Unlock()
 	server.motdLines = motdLines
+	server.configurableStateMutex.Unlock()
 	return nil
 }
 
@@ -1628,8 +1643,9 @@ func awayHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 	if len(msg.Params) > 0 {
 		isAway = true
 		text = msg.Params[0]
-		if len(text) > server.limits.AwayLen {
-			text = text[:server.limits.AwayLen]
+		awayLen := server.getLimits().AwayLen
+		if len(text) > awayLen {
+			text = text[:awayLen]
 		}
 	}