From 23a66fa5025c89034238cfa8b0a419913ad84f79 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Mon, 2 Oct 2017 04:42:50 -0400 Subject: [PATCH] fix various data races, including 2 introduced by #139 --- irc/client.go | 11 ++++--- irc/getters.go | 22 +++++++++++++ irc/isupport.go | 2 +- irc/nickname.go | 2 +- irc/server.go | 88 +++++++++++++++++++++++++++++-------------------- 5 files changed, 82 insertions(+), 43 deletions(-) create mode 100644 irc/getters.go diff --git a/irc/client.go b/irc/client.go index 04172f97..46489a92 100644 --- a/irc/client.go +++ b/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) diff --git a/irc/getters.go b/irc/getters.go new file mode 100644 index 00000000..11939ada --- /dev/null +++ b/irc/getters.go @@ -0,0 +1,22 @@ +// Copyright (c) 2017 Shivaram Lingamneni +// 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 +} diff --git a/irc/isupport.go b/irc/isupport.go index 17188c32..3e7b8edc 100644 --- a/irc/isupport.go +++ b/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...)...) } diff --git a/irc/nickname.go b/irc/nickname.go index 6de714c9..ed4b6478 100644 --- a/irc/nickname.go +++ b/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 } diff --git a/irc/server.go b/irc/server.go index 7d9feabf..f271b5fb 100644 --- a/irc/server.go +++ b/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] } }