From fa83ccd82b452772fa2e13c8a949efa4e1da24ef Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 22 Oct 2017 19:50:16 -0400 Subject: [PATCH] refactor synchronization for Channel --- irc/channel.go | 385 +++++++++++++++++++++------------------ irc/chanserv.go | 5 +- irc/client.go | 42 ++++- irc/client_lookup_set.go | 70 +++++-- irc/getters.go | 94 ++++++++++ irc/modes.go | 76 +++----- irc/roleplay.go | 4 +- irc/server.go | 90 +++------ irc/types.go | 10 - 9 files changed, 434 insertions(+), 342 deletions(-) diff --git a/irc/channel.go b/irc/channel.go index b16cc834..1fab08d8 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -23,12 +23,13 @@ type Channel struct { flags ModeSet lists map[Mode]*UserMaskSet key string - membersMutex sync.RWMutex members MemberSet + membersCache []*Client // allow iteration over channel members without holding the lock name string nameCasefolded string server *Server createdTime time.Time + stateMutex sync.RWMutex topic string topicSetBy string topicSetTime time.Time @@ -68,28 +69,29 @@ func NewChannel(s *Server, name string, addDefaultModes bool) *Channel { return channel } -// IsEmpty returns true if the channel has no clients. -func (channel *Channel) IsEmpty() bool { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() +func (channel *Channel) regenerateMembersCache() { + // this is eventually consistent even without holding the writable Lock() + // throughout the update; all updates to `members` while holding Lock() + // have a serial order, so the call to `regenerateMembersCache` that + // happens-after the last one will see *all* the updates + channel.stateMutex.RLock() + result := make([]*Client, len(channel.members)) + i := 0 + for client := range channel.members { + result[i] = client + i++ + } + channel.stateMutex.RUnlock() + channel.stateMutex.Lock() + channel.membersCache = result + channel.stateMutex.Unlock() + return - return channel.isEmptyNoMutex() -} - -func (channel *Channel) isEmptyNoMutex() bool { - return len(channel.members) == 0 } // Names sends the list of users joined to the channel to the given client. func (channel *Channel) Names(client *Client) { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - - channel.namesNoMutex(client) -} - -func (channel *Channel) namesNoMutex(client *Client) { - currentNicks := channel.nicksNoMutex(client) + currentNicks := channel.nicks(client) // assemble and send replies maxNamLen := 480 - len(client.server.name) - len(client.nick) var buffer string @@ -115,14 +117,8 @@ func (channel *Channel) namesNoMutex(client *Client) { // ClientIsAtLeast returns whether the client has at least the given channel privilege. func (channel *Channel) ClientIsAtLeast(client *Client, permission Mode) bool { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - - return channel.clientIsAtLeastNoMutex(client, permission) -} - -func (channel *Channel) clientIsAtLeastNoMutex(client *Client, permission Mode) bool { - // requires RLock() + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() // get voice, since it's not a part of ChannelPrivModes if channel.members.HasMode(client, permission) { @@ -164,60 +160,119 @@ func (modes ModeSet) Prefixes(isMultiPrefix bool) string { return prefixes } -func (channel *Channel) nicksNoMutex(target *Client) []string { +func (channel *Channel) ClientPrefixes(client *Client, isMultiPrefix bool) string { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + modes, present := channel.members[client] + if !present { + return "" + } else { + return modes.Prefixes(isMultiPrefix) + } +} + +func (channel *Channel) ClientHasPrivsOver(client *Client, target *Client) bool { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + + clientModes := channel.members[client] + targetModes := channel.members[target] + result := false + for _, mode := range ChannelPrivModes { + if clientModes[mode] { + result = true + // admins cannot kick other admins + if mode == ChannelAdmin && targetModes[ChannelAdmin] { + result = false + } + break + } else if channel.members[target][mode] { + break + } + } + return result +} + +func (channel *Channel) nicks(target *Client) []string { isMultiPrefix := (target != nil) && target.capabilities.Has(caps.MultiPrefix) isUserhostInNames := (target != nil) && target.capabilities.Has(caps.UserhostInNames) - nicks := make([]string, len(channel.members)) + + // slightly cumbersome: get the mutex and copy both the client pointers and + // the mode prefixes + channel.stateMutex.RLock() + length := len(channel.members) + clients := make([]*Client, length) + result := make([]string, length) i := 0 for client, modes := range channel.members { - nicks[i] += modes.Prefixes(isMultiPrefix) + clients[i] = client + result[i] = modes.Prefixes(isMultiPrefix) + i++ + } + channel.stateMutex.RUnlock() + + i = 0 + for i < length { if isUserhostInNames { - nicks[i] += client.nickMaskString + result[i] += clients[i].getNickMaskString() } else { - nicks[i] += client.nick + result[i] += clients[i].getNick() } i++ } - return nicks + + return result +} + +func (channel *Channel) hasClient(client *Client) bool { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + _, present := channel.members[client] + return present } // -func (channel *Channel) modeStringNoLock(client *Client) (str string) { - // RLock() - isMember := client.flags[Operator] || channel.members.Has(client) - // RUnlock() +func (channel *Channel) modeStrings(client *Client) (result []string) { + isMember := client.HasMode(Operator) || channel.hasClient(client) showKey := isMember && (channel.key != "") showUserLimit := channel.userLimit > 0 + modes := "+" + // flags with args if showKey { - str += Key.String() + modes += Key.String() } if showUserLimit { - str += UserLimit.String() + modes += UserLimit.String() } + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + // flags for mode := range channel.flags { - str += mode.String() + modes += mode.String() } - str = "+" + str + result = []string{modes} // args for flags with args: The order must match above to keep // positional arguments in place. if showKey { - str += " " + channel.key + result = append(result, channel.key) } if showUserLimit { - str += " " + strconv.FormatUint(channel.userLimit, 10) + result = append(result, strconv.FormatUint(channel.userLimit, 10)) } - return str + return } // IsFull returns true if this channel is at its' members limit. func (channel *Channel) IsFull() bool { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() return (channel.userLimit > 0) && (uint64(len(channel.members)) >= channel.userLimit) } @@ -229,9 +284,7 @@ func (channel *Channel) CheckKey(key string) bool { // Join joins the given client to this channel (if they can be joined). //TODO(dan): /SAJOIN and maybe a ForceJoin function? func (channel *Channel) Join(client *Client, key string) { - channel.membersMutex.Lock() - defer channel.membersMutex.Unlock() - if channel.members.Has(client) { + if channel.hasClient(client) { // already joined, no message needs to be sent return } @@ -261,7 +314,7 @@ func (channel *Channel) Join(client *Client, key string) { client.server.logger.Debug("join", fmt.Sprintf("%s joined channel %s", client.nick, channel.name)) - for member := range channel.members { + for _, member := range channel.Members() { if member.capabilities.Has(caps.ExtendedJoin) { member.Send(nil, client.nickMaskString, "JOIN", channel.name, client.account.Name, client.realname) } else { @@ -269,8 +322,13 @@ func (channel *Channel) Join(client *Client, key string) { } } - client.channels.Add(channel) + channel.stateMutex.Lock() channel.members.Add(client) + firstJoin := len(channel.members) == 1 + channel.stateMutex.Unlock() + channel.regenerateMembersCache() + + client.addChannel(channel) // give channel mode if necessary var newChannel bool @@ -281,20 +339,25 @@ func (channel *Channel) Join(client *Client, key string) { chanReg := client.server.loadChannelNoMutex(tx, channel.nameCasefolded) if chanReg == nil { - if len(channel.members) == 1 { + if firstJoin { + channel.stateMutex.Lock() channel.createdTime = time.Now() channel.members[client][ChannelOperator] = true + channel.stateMutex.Unlock() givenMode = &ChannelOperator newChannel = true } } else { // we should only do this on registered channels if client.account != nil && client.account.Name == chanReg.Founder { + channel.stateMutex.Lock() channel.members[client][ChannelFounder] = true + channel.stateMutex.Unlock() givenMode = &ChannelFounder } - if len(channel.members) == 1 { + if firstJoin { // apply other details if new channel + channel.stateMutex.Lock() channel.topic = chanReg.Topic channel.topicSetBy = chanReg.TopicSetBy channel.topicSetTime = chanReg.TopicSetTime @@ -309,6 +372,7 @@ func (channel *Channel) Join(client *Client, key string) { for _, mask := range chanReg.Invitelist { channel.lists[InviteMask].Add(mask) } + channel.stateMutex.Unlock() } } return nil @@ -321,11 +385,11 @@ func (channel *Channel) Join(client *Client, key string) { } // don't sent topic when it's an entirely new channel if !newChannel { - channel.getTopicNoMutex(client) // we already have Lock + channel.SendTopic(client) } - channel.namesNoMutex(client) + channel.Names(client) if givenMode != nil { - for member := range channel.members { + for _, member := range channel.Members() { member.Send(nil, client.server.name, "MODE", channel.name, fmt.Sprintf("+%v", *givenMode), client.nick) } } @@ -333,58 +397,50 @@ func (channel *Channel) Join(client *Client, key string) { // Part parts the given client from this channel, with the given message. func (channel *Channel) Part(client *Client, message string) { - channel.membersMutex.Lock() - defer channel.membersMutex.Unlock() - - if !channel.members.Has(client) { + if !channel.hasClient(client) { client.Send(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, "You're not on that channel") return } - for member := range channel.members { + for _, member := range channel.Members() { member.Send(nil, client.nickMaskString, "PART", channel.name, message) } - channel.quitNoMutex(client) + channel.Quit(client) client.server.logger.Debug("part", fmt.Sprintf("%s left channel %s", client.nick, channel.name)) } -// GetTopic sends the channel topic to the given client. -func (channel *Channel) GetTopic(client *Client) { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - - channel.getTopicNoMutex(client) -} - -// GetTopic sends the channel topic to the given client without getting the membersMutex. -// This is required because of channel joins. -func (channel *Channel) getTopicNoMutex(client *Client) { - if !channel.members.Has(client) { +// SendTopic sends the channel topic to the given client. +func (channel *Channel) SendTopic(client *Client) { + if !channel.hasClient(client) { client.Send(nil, client.server.name, ERR_NOTONCHANNEL, client.nick, channel.name, "You're not on that channel") return } - if channel.topic == "" { - client.Send(nil, client.server.name, RPL_NOTOPIC, client.nick, channel.name, "No topic is set") + channel.stateMutex.RLock() + name := channel.name + topic := channel.topic + topicSetBy := channel.topicSetBy + topicSetTime := channel.topicSetTime + channel.stateMutex.RUnlock() + + if topic == "" { + client.Send(nil, client.server.name, RPL_NOTOPIC, client.nick, name, "No topic is set") return } - client.Send(nil, client.server.name, RPL_TOPIC, client.nick, channel.name, channel.topic) - client.Send(nil, client.server.name, RPL_TOPICTIME, client.nick, channel.name, channel.topicSetBy, strconv.FormatInt(channel.topicSetTime.Unix(), 10)) + client.Send(nil, client.server.name, RPL_TOPIC, client.nick, name, topic) + client.Send(nil, client.server.name, RPL_TOPICTIME, client.nick, name, topicSetBy, strconv.FormatInt(topicSetTime.Unix(), 10)) } // SetTopic sets the topic of this channel, if the client is allowed to do so. func (channel *Channel) SetTopic(client *Client, topic string) { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - - if !(client.flags[Operator] || channel.members.Has(client)) { + if !(client.flags[Operator] || channel.hasClient(client)) { client.Send(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, "You're not on that channel") return } - if channel.flags[OpOnlyTopic] && !channel.ClientIsAtLeast(client, ChannelOperator) { + if channel.HasMode(OpOnlyTopic) && !channel.ClientIsAtLeast(client, ChannelOperator) { client.Send(nil, client.server.name, ERR_CHANOPRIVSNEEDED, channel.name, "You're not a channel operator") return } @@ -393,12 +449,14 @@ func (channel *Channel) SetTopic(client *Client, topic string) { topic = topic[:client.server.limits.TopicLen] } + channel.stateMutex.Lock() channel.topic = topic channel.topicSetBy = client.nickMaskString channel.topicSetTime = time.Now() + channel.stateMutex.Unlock() - for member := range channel.members { - member.Send(nil, client.nickMaskString, "TOPIC", channel.name, channel.topic) + for _, member := range channel.Members() { + member.Send(nil, client.nickMaskString, "TOPIC", channel.name, topic) } // update saved channel topic for registered chans @@ -422,13 +480,14 @@ func (channel *Channel) SetTopic(client *Client, topic string) { // CanSpeak returns true if the client can speak on this channel. func (channel *Channel) CanSpeak(client *Client) bool { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() - if channel.flags[NoOutside] && !channel.members.Has(client) { + _, hasClient := channel.members[client] + if channel.flags[NoOutside] && !hasClient { return false } - if channel.flags[Moderated] && !channel.clientIsAtLeastNoMutex(client, Voice) { + if channel.flags[Moderated] && !channel.ClientIsAtLeast(client, Voice) { return false } if channel.flags[RegisteredOnly] && client.account == &NoAccount { @@ -449,15 +508,12 @@ func (channel *Channel) sendMessage(msgid, cmd string, requiredCaps []caps.Capab return } - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - // for STATUSMSG var minPrefixMode Mode if minPrefix != nil { minPrefixMode = *minPrefix } - for member := range channel.members { + for _, member := range channel.Members() { if minPrefix != nil && !channel.ClientIsAtLeast(member, minPrefixMode) { // STATUSMSG continue @@ -505,15 +561,12 @@ func (channel *Channel) sendSplitMessage(msgid, cmd string, minPrefix *Mode, cli return } - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - // for STATUSMSG var minPrefixMode Mode if minPrefix != nil { minPrefixMode = *minPrefix } - for member := range channel.members { + for _, member := range channel.Members() { if minPrefix != nil && !channel.ClientIsAtLeast(member, minPrefixMode) { // STATUSMSG continue @@ -534,35 +587,8 @@ func (channel *Channel) sendSplitMessage(msgid, cmd string, minPrefix *Mode, cli } } -func (channel *Channel) applyModeFlag(client *Client, mode Mode, - op ModeOp) bool { - if !channel.ClientIsAtLeast(client, ChannelOperator) { - client.Send(nil, client.server.name, ERR_CHANOPRIVSNEEDED, channel.name, "You're not a channel operator") - return false - } - - switch op { - case Add: - if channel.flags[mode] { - return false - } - channel.flags[mode] = true - return true - - case Remove: - if !channel.flags[mode] { - return false - } - delete(channel.flags, mode) - return true - } - return false -} - func (channel *Channel) applyModeMemberNoMutex(client *Client, mode Mode, op ModeOp, nick string) *ModeChange { - // requires Lock() - if nick == "" { //TODO(dan): shouldn't this be handled before it reaches this function? client.Send(nil, client.server.name, ERR_NEEDMOREPARAMS, "MODE", "Not enough parameters") @@ -576,35 +602,28 @@ func (channel *Channel) applyModeMemberNoMutex(client *Client, mode Mode, return nil } - if !channel.members.Has(target) { + channel.stateMutex.Lock() + modeset, exists := channel.members[target] + var already bool + if exists { + enable := op == Add + already = modeset[mode] == enable + modeset[mode] = enable + } + channel.stateMutex.Unlock() + + if !exists { client.Send(nil, client.server.name, ERR_USERNOTINCHANNEL, client.nick, channel.name, "They aren't on that channel") return nil - } - - switch op { - case Add: - if channel.members[target][mode] { - return nil - } - channel.members[target][mode] = true + } else if already { + return nil + } else { return &ModeChange{ - op: Add, - mode: mode, - arg: nick, - } - - case Remove: - if !channel.members[target][mode] { - return nil - } - channel.members[target][mode] = false - return &ModeChange{ - op: Remove, + op: op, mode: mode, arg: nick, } } - return nil } // ShowMaskList shows the given list to the client. @@ -622,11 +641,15 @@ func (channel *Channel) ShowMaskList(client *Client, mode Mode) { rplendoflist = RPL_ENDOFINVITELIST } - // send out responses + nick := client.getNick() + channel.stateMutex.RLock() + // XXX don't acquire any new locks in this section, besides Socket.Write for mask := range channel.lists[mode].masks { - client.Send(nil, client.server.name, rpllist, client.nick, channel.name, mask) + client.Send(nil, client.server.name, rpllist, nick, channel.name, mask) } - client.Send(nil, client.server.name, rplendoflist, client.nick, channel.name, "End of list") + channel.stateMutex.RUnlock() + + client.Send(nil, client.server.name, rplendoflist, nick, channel.name, "End of list") } func (channel *Channel) applyModeMask(client *Client, mode Mode, op ModeOp, mask string) bool { @@ -657,51 +680,52 @@ func (channel *Channel) applyModeMask(client *Client, mode Mode, op ModeOp, mask return false } -// Quit removes the given client from the channel, and also updates friends with the latest client list. -func (channel *Channel) Quit(client *Client, friends *ClientSet) { - channel.membersMutex.Lock() - defer channel.membersMutex.Unlock() - - channel.quitNoMutex(client) - - for friend := range channel.members { - friends.Add(friend) - } -} - -func (channel *Channel) quitNoMutex(client *Client) { +// Quit removes the given client from the channel +func (channel *Channel) Quit(client *Client) { + channel.stateMutex.Lock() channel.members.Remove(client) - client.channels.Remove(channel) + empty := len(channel.members) == 0 + channel.stateMutex.Unlock() + channel.regenerateMembersCache() - if channel.isEmptyNoMutex() { + client.removeChannel(channel) + + //TODO(slingamn) fold this operation into a channelmanager type + if empty { channel.server.channels.Remove(channel) } } -func (channel *Channel) kickNoMutex(client *Client, target *Client, comment string) { - // needs a Lock() - - if !(client.flags[Operator] || channel.members.Has(client)) { +func (channel *Channel) Kick(client *Client, target *Client, comment string) { + if !(client.flags[Operator] || channel.hasClient(client)) { client.Send(nil, client.server.name, ERR_NOTONCHANNEL, channel.name, "You're not on that channel") return } - if !channel.clientIsAtLeastNoMutex(client, ChannelOperator) { + if !channel.ClientIsAtLeast(client, ChannelOperator) { client.Send(nil, client.server.name, ERR_CANNOTSENDTOCHAN, channel.name, "Cannot send to channel") return } - if !channel.members.Has(target) { + if !channel.hasClient(target) { client.Send(nil, client.server.name, ERR_USERNOTINCHANNEL, client.nick, channel.name, "They aren't on that channel") return } - - if len(comment) > client.server.limits.KickLen { - comment = comment[:client.server.limits.KickLen] + if !channel.ClientHasPrivsOver(client, target) { + client.Send(nil, client.server.name, ERR_CHANOPRIVSNEEDED, channel.name, "You're not a channel operator") + return } - for member := range channel.members { - member.Send(nil, client.nickMaskString, "KICK", channel.name, target.nick, comment) + kicklimit := client.server.getLimits().KickLen + if len(comment) > kicklimit { + comment = comment[:kicklimit] } - channel.quitNoMutex(target) + + clientMask := client.getNickMaskString() + targetNick := target.getNick() + for _, member := range channel.Members() { + member.Send(nil, clientMask, "KICK", channel.name, targetNick, comment) + } + + channel.Quit(target) } // Invite invites the given client to the channel, if the inviter can do so. @@ -711,23 +735,22 @@ func (channel *Channel) Invite(invitee *Client, inviter *Client) { return } - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - - if !channel.members.Has(inviter) { + if !channel.hasClient(inviter) { inviter.Send(nil, inviter.server.name, ERR_NOTONCHANNEL, channel.name, "You're not on that channel") return } //TODO(dan): handle this more nicely, keep a list of last X invited channels on invitee rather than explicitly modifying the invite list? if channel.flags[InviteOnly] { - channel.lists[InviteMask].Add(invitee.nickMaskCasefolded) + nmc := invitee.getNickCasefolded() + channel.stateMutex.Lock() + channel.lists[InviteMask].Add(nmc) + channel.stateMutex.Unlock() } - // send invite-notify - for member := range channel.members { + for _, member := range channel.Members() { if member.capabilities.Has(caps.InviteNotify) && member != inviter && member != invitee && channel.ClientIsAtLeast(member, Halfop) { - member.Send(nil, inviter.nickMaskString, "INVITE", invitee.nick, channel.name) + member.Send(nil, inviter.getNickMaskString(), "INVITE", invitee.getNick(), channel.name) } } diff --git a/irc/chanserv.go b/irc/chanserv.go index 99f0ba7a..eb9523c4 100644 --- a/irc/chanserv.go +++ b/irc/chanserv.go @@ -105,16 +105,13 @@ func (server *Server) chanservReceivePrivmsg(client *Client, message string) { server.logger.Info("chanserv", fmt.Sprintf("Client %s registered channel %s", client.nick, channelName)) server.snomasks.Send(sno.LocalChannels, fmt.Sprintf(ircfmt.Unescape("Channel registered $c[grey][$r%s$c[grey]] by $c[grey][$r%s$c[grey]]"), channelName, client.nickMaskString)) - channelInfo.membersMutex.Lock() - defer channelInfo.membersMutex.Unlock() - // give them founder privs change := channelInfo.applyModeMemberNoMutex(client, ChannelFounder, Add, client.nickCasefolded) if change != nil { //TODO(dan): we should change the name of String and make it return a slice here //TODO(dan): unify this code with code in modes.go args := append([]string{channelName}, strings.Split(change.String(), " ")...) - for member := range channelInfo.members { + for _, member := range channelInfo.Members() { member.Send(nil, fmt.Sprintf("ChanServ!services@%s", client.server.name), "MODE", args...) } } diff --git a/irc/client.go b/irc/client.go index f78ed23e..91186e4b 100644 --- a/irc/client.go +++ b/irc/client.go @@ -14,6 +14,7 @@ import ( "strconv" "strings" "sync" + "sync/atomic" "time" "github.com/goshuirc/irc-go/ircfmt" @@ -55,6 +56,8 @@ type Client struct { idletimer *IdleTimer isDestroyed bool isQuitting bool + maxlenTags uint32 + maxlenRest uint32 nick string nickCasefolded string nickMaskCasefolded string @@ -162,7 +165,7 @@ func (client *Client) IPString() string { // command goroutine // -func (client *Client) maxlens() (int, int) { +func (client *Client) recomputeMaxlens() (int, int) { maxlenTags := 512 maxlenRest := 512 if client.capabilities.Has(caps.MessageTags) { @@ -175,9 +178,19 @@ func (client *Client) maxlens() (int, int) { } maxlenRest = limits.LineLen.Rest } + + atomic.StoreUint32(&client.maxlenTags, uint32(maxlenTags)) + atomic.StoreUint32(&client.maxlenRest, uint32(maxlenRest)) + return maxlenTags, maxlenRest } +// allow these negotiated length limits to be read without locks; this is a convenience +// so that Client.Send doesn't have to acquire any Client locks +func (client *Client) maxlens() (int, int) { + return int(atomic.LoadUint32(&client.maxlenTags)), int(atomic.LoadUint32(&client.maxlenRest)) +} + func (client *Client) run() { var err error var isExiting bool @@ -192,14 +205,14 @@ func (client *Client) run() { client.rawHostname = utils.AddrLookupHostname(client.socket.conn.RemoteAddr()) for { + maxlenTags, maxlenRest := client.recomputeMaxlens() + line, err = client.socket.Read() if err != nil { client.Quit("connection closed") break } - maxlenTags, maxlenRest := client.maxlens() - client.server.logger.Debug("userinput ", client.nick, "<- ", line) msg, err = ircmsg.ParseLineMaxLen(line, maxlenTags, maxlenRest) @@ -338,9 +351,8 @@ func (client *Client) Friends(capabs ...caps.Capability) ClientSet { friends.Add(client) } - for channel := range client.channels { - channel.membersMutex.RLock() - for member := range channel.members { + for _, channel := range client.Channels() { + for _, member := range channel.Members() { // make sure they have all the required caps hasCaps = true for _, capab := range capabs { @@ -353,7 +365,6 @@ func (client *Client) Friends(capabs ...caps.Capability) ClientSet { friends.Add(member) } } - channel.membersMutex.RUnlock() } return friends } @@ -527,7 +538,10 @@ func (client *Client) destroy() { // clean up channels client.server.channelJoinPartMutex.Lock() for channel := range client.channels { - channel.Quit(client, &friends) + channel.Quit(client) + for _, member := range channel.Members() { + friends.Add(member) + } } client.server.channelJoinPartMutex.Unlock() @@ -663,3 +677,15 @@ func (client *Client) Notice(text string) { client.Send(nil, client.server.name, "NOTICE", client.nick, line) } } + +func (client *Client) addChannel(channel *Channel) { + client.stateMutex.Lock() + client.channels[channel] = true + client.stateMutex.Unlock() +} + +func (client *Client) removeChannel(channel *Channel) { + client.stateMutex.Lock() + delete(client.channels, channel) + client.stateMutex.Unlock() +} diff --git a/irc/client_lookup_set.go b/irc/client_lookup_set.go index 7c0070e3..66919224 100644 --- a/irc/client_lookup_set.go +++ b/irc/client_lookup_set.go @@ -227,6 +227,7 @@ func (clients *ClientLookupSet) Find(userhost string) *Client { // UserMaskSet holds a set of client masks and lets you match hostnames to them. type UserMaskSet struct { + sync.RWMutex masks map[string]bool regexp *regexp.Regexp } @@ -245,16 +246,25 @@ func (set *UserMaskSet) Add(mask string) bool { log.Println(fmt.Sprintf("ERROR: Could not add mask to usermaskset: [%s]", mask)) return false } - if set.masks[casefoldedMask] { - return false - } + + set.Lock() + already := set.masks[casefoldedMask] set.masks[casefoldedMask] = true - set.setRegexp() - return true + set.Unlock() + + if already { + return false + } else { + set.setRegexp() + return true + } } // AddAll adds the given masks to this set. func (set *UserMaskSet) AddAll(masks []string) (added bool) { + set.Lock() + defer set.Unlock() + for _, mask := range masks { if !added && !set.masks[mask] { added = true @@ -267,33 +277,50 @@ func (set *UserMaskSet) AddAll(masks []string) (added bool) { // Remove removes the given mask from this set. func (set *UserMaskSet) Remove(mask string) bool { - if !set.masks[mask] { - return false - } + set.Lock() + already := !set.masks[mask] delete(set.masks, mask) - set.setRegexp() - return true + set.Unlock() + + if !already { + return false + } else { + set.setRegexp() + return true + } } // Match matches the given n!u@h. func (set *UserMaskSet) Match(userhost string) bool { - if set.regexp == nil { + set.RLock() + regexp := set.regexp + set.RUnlock() + + if regexp == nil { return false } - return set.regexp.MatchString(userhost) + return regexp.MatchString(userhost) } // String returns the masks in this set. func (set *UserMaskSet) String() string { + set.RLock() masks := make([]string, len(set.masks)) index := 0 for mask := range set.masks { masks[index] = mask index++ } + set.RUnlock() return strings.Join(masks, " ") } +func (set *UserMaskSet) Length() int { + set.RLock() + defer set.RUnlock() + return len(set.masks) +} + // setRegexp generates a regular expression from the set of user mask // strings. Masks are split at the two types of wildcards, `*` and // `?`. All the pieces are meta-escaped. `*` is replaced with `.*`, @@ -301,11 +328,9 @@ func (set *UserMaskSet) String() string { // parts are re-joined and finally all masks are joined into a big // or-expression. func (set *UserMaskSet) setRegexp() { - if len(set.masks) == 0 { - set.regexp = nil - return - } + var re *regexp.Regexp + set.RLock() maskExprs := make([]string, len(set.masks)) index := 0 for mask := range set.masks { @@ -320,7 +345,16 @@ func (set *UserMaskSet) setRegexp() { manyExprs[mindex] = strings.Join(oneExprs, ".") } maskExprs[index] = strings.Join(manyExprs, ".*") + index++ } - expr := "^" + strings.Join(maskExprs, "|") + "$" - set.regexp, _ = regexp.Compile(expr) + set.RUnlock() + + if index > 0 { + expr := "^" + strings.Join(maskExprs, "|") + "$" + re, _ = regexp.Compile(expr) + } + + set.Lock() + set.regexp = re + set.Unlock() } diff --git a/irc/getters.go b/irc/getters.go index e76938d4..5ae83c41 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -53,6 +53,24 @@ func (client *Client) getNickCasefolded() string { return client.nickCasefolded } +func (client *Client) Username() string { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + return client.username +} + +func (client *Client) Hostname() string { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + return client.hostname +} + +func (client *Client) Realname() string { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + return client.realname +} + func (client *Client) Registered() bool { client.stateMutex.RLock() defer client.stateMutex.RUnlock() @@ -64,3 +82,79 @@ func (client *Client) Destroyed() bool { defer client.stateMutex.RUnlock() return client.isDestroyed } + +func (client *Client) HasMode(mode Mode) bool { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + return client.flags[mode] +} + +func (client *Client) Channels() (result []*Channel) { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + length := len(client.channels) + result = make([]*Channel, length) + i := 0 + for channel := range client.channels { + result[i] = channel + i++ + } + return +} + +func (channel *Channel) Name() string { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + return channel.name +} + +func (channel *Channel) Members() (result []*Client) { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + return channel.membersCache +} + +func (channel *Channel) UserLimit() uint64 { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + return channel.userLimit +} + +func (channel *Channel) setUserLimit(limit uint64) { + channel.stateMutex.Lock() + channel.userLimit = limit + channel.stateMutex.Unlock() +} + +func (channel *Channel) Key() string { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + return channel.key +} + +func (channel *Channel) setKey(key string) { + channel.stateMutex.Lock() + channel.key = key + channel.stateMutex.Unlock() +} + +func (channel *Channel) HasMode(mode Mode) bool { + channel.stateMutex.RLock() + defer channel.stateMutex.RUnlock() + return channel.flags[mode] +} + +// set a channel mode, return whether it was already set +func (channel *Channel) setMode(mode Mode, enable bool) (already bool) { + channel.stateMutex.Lock() + already = (channel.flags[mode] == enable) + if !already { + if enable { + channel.flags[mode] = true + } else { + delete(channel.flags, mode) + } + } + channel.stateMutex.Unlock() + return +} diff --git a/irc/modes.go b/irc/modes.go index c910c53d..31ea0003 100644 --- a/irc/modes.go +++ b/irc/modes.go @@ -475,11 +475,11 @@ func ParseChannelModeChanges(params ...string) (ModeChanges, map[rune]bool) { } // ApplyChannelModeChanges applies a given set of mode changes. -func ApplyChannelModeChanges(channel *Channel, client *Client, isSamode bool, changes ModeChanges) ModeChanges { +func (channel *Channel) ApplyChannelModeChanges(client *Client, isSamode bool, changes ModeChanges) ModeChanges { // so we only output one warning for each list type when full listFullWarned := make(map[Mode]bool) - clientIsOp := channel.clientIsAtLeastNoMutex(client, ChannelOperator) + clientIsOp := channel.ClientIsAtLeast(client, ChannelOperator) var alreadySentPrivError bool applied := make(ModeChanges, 0) @@ -498,12 +498,6 @@ func ApplyChannelModeChanges(channel *Channel, client *Client, isSamode bool, ch switch change.mode { case BanMask, ExceptMask, InviteMask: mask := change.arg - list := channel.lists[change.mode] - if list == nil { - // This should never happen, but better safe than panicky. - client.Send(nil, client.server.name, ERR_UNKNOWNERROR, client.nick, "MODE", "Could not complete MODE command") - return changes - } if (change.op == List) || (mask == "") { channel.ShowMaskList(client, change.mode) @@ -518,19 +512,19 @@ func ApplyChannelModeChanges(channel *Channel, client *Client, isSamode bool, ch switch change.op { case Add: - if len(list.masks) >= client.server.limits.ChanListModes { + if channel.lists[change.mode].Length() >= client.server.getLimits().ChanListModes { if !listFullWarned[change.mode] { - client.Send(nil, client.server.name, ERR_BANLISTFULL, client.nick, channel.name, change.mode.String(), "Channel list is full") + client.Send(nil, client.server.name, ERR_BANLISTFULL, client.getNick(), channel.Name(), change.mode.String(), "Channel list is full") listFullWarned[change.mode] = true } continue } - list.Add(mask) + channel.lists[change.mode].Add(mask) applied = append(applied, change) case Remove: - list.Remove(mask) + channel.lists[change.mode].Remove(mask) applied = append(applied, change) } @@ -539,62 +533,46 @@ func ApplyChannelModeChanges(channel *Channel, client *Client, isSamode bool, ch case Add: val, err := strconv.ParseUint(change.arg, 10, 64) if err == nil { - channel.userLimit = val + channel.setUserLimit(val) applied = append(applied, change) } case Remove: - channel.userLimit = 0 + channel.setUserLimit(0) applied = append(applied, change) } case Key: switch change.op { case Add: - channel.key = change.arg + channel.setKey(change.arg) case Remove: - channel.key = "" + channel.setKey("") } applied = append(applied, change) case InviteOnly, Moderated, NoOutside, OpOnlyTopic, RegisteredOnly, Secret, ChanRoleplaying: - switch change.op { - case Add: - if channel.flags[change.mode] { - continue - } - channel.flags[change.mode] = true - applied = append(applied, change) + if change.op == List { + continue + } - case Remove: - if !channel.flags[change.mode] { - continue - } - delete(channel.flags, change.mode) + already := channel.setMode(change.mode, change.op == Add) + if !already { applied = append(applied, change) } case ChannelFounder, ChannelAdmin, ChannelOperator, Halfop, Voice: + if change.op == List { + continue + } // make sure client has privs to edit the given prefix hasPrivs := isSamode - if !hasPrivs { - for _, mode := range ChannelPrivModes { - if channel.members[client][mode] { - hasPrivs = true - - // Admins can't give other people Admin or remove it from others, - // standard for that channel mode, we worry about this later - if mode == ChannelAdmin && change.mode == ChannelAdmin { - hasPrivs = false - } - - break - } else if mode == change.mode { - break - } - } + // Admins can't give other people Admin or remove it from others, + // standard for that channel mode, we worry about this later + if !hasPrivs && change.mode != ChannelAdmin { + hasPrivs = channel.ClientIsAtLeast(client, change.mode) } casefoldedName, err := CasefoldName(change.arg) @@ -634,9 +612,6 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { return false } - channel.membersMutex.Lock() - defer channel.membersMutex.Unlock() - // applied mode changes applied := make(ModeChanges, 0) @@ -654,7 +629,7 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { } // apply mode changes - applied = ApplyChannelModeChanges(channel, client, msg.Command == "SAMODE", changes) + applied = channel.ApplyChannelModeChanges(client, msg.Command == "SAMODE", changes) } // save changes to banlist/exceptlist/invexlist @@ -707,12 +682,11 @@ func cmodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { if len(applied) > 0 { //TODO(dan): we should change the name of String and make it return a slice here args := append([]string{channel.name}, strings.Split(applied.String(), " ")...) - for member := range channel.members { + for _, member := range channel.Members() { member.Send(nil, client.nickMaskString, "MODE", args...) } } else { - //TODO(dan): we should just make ModeString return a slice here - args := append([]string{client.nick, channel.name}, strings.Split(channel.modeStringNoLock(client), " ")...) + args := append([]string{client.nick, channel.name}, channel.modeStrings(client)...) client.Send(nil, client.nickMaskString, RPL_CHANNELMODEIS, args...) client.Send(nil, client.nickMaskString, RPL_CHANNELCREATED, client.nick, channel.name, strconv.FormatInt(channel.createdTime.Unix(), 10)) } diff --git a/irc/roleplay.go b/irc/roleplay.go index d5d14d4d..218950a8 100644 --- a/irc/roleplay.go +++ b/irc/roleplay.go @@ -88,14 +88,12 @@ func sendRoleplayMessage(server *Server, client *Client, source string, targetSt return } - channel.membersMutex.RLock() - for member := range channel.members { + for _, member := range channel.Members() { if member == client && !client.capabilities.Has(caps.EchoMessage) { continue } member.Send(nil, source, "PRIVMSG", channel.name, message) } - channel.membersMutex.RUnlock() } else { target, err := CasefoldName(targetString) user := server.clients.Get(target) diff --git a/irc/server.go b/irc/server.go index 64432f15..b6963754 100644 --- a/irc/server.go +++ b/irc/server.go @@ -587,9 +587,6 @@ func renameHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { return false } - channel.membersMutex.Lock() - defer channel.membersMutex.Unlock() - casefoldedNewName, err := CasefoldChannel(newName) if err != nil { //TODO(dan): Change this to ERR_CANNOTRENAME @@ -646,7 +643,7 @@ func renameHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { }) // send RENAME messages - for mcl := range channel.members { + for _, mcl := range channel.Members() { if mcl.capabilities.Has(caps.Rename) { mcl.Send(nil, client.nickMaskString, "RENAME", oldName, newName, reason) } else { @@ -755,7 +752,7 @@ func topicHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { if len(msg.Params) > 1 { channel.SetTopic(client, msg.Params[1]) } else { - channel.GetTopic(client) + channel.SendTopic(client) } return false } @@ -964,17 +961,12 @@ func tagmsgHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { func (client *Client) WhoisChannelsNames(target *Client) []string { isMultiPrefix := target.capabilities.Has(caps.MultiPrefix) var chstrs []string - index := 0 - for channel := range client.channels { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - + for _, channel := range client.Channels() { // channel is secret and the target can't see it - if !target.flags[Operator] && channel.flags[Secret] && !channel.members.Has(target) { + if !target.flags[Operator] && channel.HasMode(Secret) && !channel.hasClient(target) { continue } - chstrs = append(chstrs, channel.members[client].Prefixes(isMultiPrefix)+channel.name) - index++ + chstrs = append(chstrs, channel.ClientPrefixes(client, isMultiPrefix)+channel.name) } return chstrs } @@ -1050,36 +1042,33 @@ func (client *Client) getWhoisOf(target *Client) { client.Send(nil, client.server.name, RPL_WHOISIDLE, client.nick, target.nick, strconv.FormatUint(target.IdleSeconds(), 10), strconv.FormatInt(target.SignonTime(), 10), "seconds idle, signon time") } -// RplWhoReplyNoMutex returns the WHO reply between one user and another channel/user. +// rplWhoReply returns the WHO reply between one user and another channel/user. // ( "H" / "G" ) ["*"] [ ( "@" / "+" ) ] // : -func (target *Client) RplWhoReplyNoMutex(channel *Channel, client *Client) { +func (target *Client) rplWhoReply(channel *Channel, client *Client) { channelName := "*" flags := "" - if client.flags[Away] { + if client.HasMode(Away) { flags = "G" } else { flags = "H" } - if client.flags[Operator] { + if client.HasMode(Operator) { flags += "*" } if channel != nil { - flags += channel.members[client].Prefixes(target.capabilities.Has(caps.MultiPrefix)) + flags += channel.ClientPrefixes(client, target.capabilities.Has(caps.MultiPrefix)) channelName = channel.name } - target.Send(nil, target.server.name, RPL_WHOREPLY, target.nick, channelName, client.username, client.hostname, client.server.name, client.nick, flags, strconv.Itoa(client.hops)+" "+client.realname) + target.Send(nil, target.server.name, RPL_WHOREPLY, target.nick, channelName, client.Username(), client.Hostname(), client.server.name, client.getNick(), flags, strconv.Itoa(client.hops)+" "+client.Realname()) } func whoChannel(client *Client, channel *Channel, friends ClientSet) { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - - for member := range channel.members { + for _, member := range channel.Members() { if !client.flags[Invisible] || friends[client] { - client.RplWhoReplyNoMutex(channel, member) + client.rplWhoReply(channel, member) } } } @@ -1120,7 +1109,7 @@ func whoHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { } } else { for mclient := range server.clients.FindAll(mask) { - client.RplWhoReplyNoMutex(nil, mclient) + client.rplWhoReply(nil, mclient) } } @@ -1792,37 +1781,10 @@ func kickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { continue } - // make sure client has privs to kick the given user - //TODO(dan): split this into a separate function that checks if users have privs - // over other users, useful for things like -aoh as well - channel.membersMutex.Lock() - - var hasPrivs bool - for _, mode := range ChannelPrivModes { - if channel.members[client][mode] { - hasPrivs = true - - // admins cannot kick other admins - if mode == ChannelAdmin && channel.members[target][ChannelAdmin] { - hasPrivs = false - } - - break - } else if channel.members[target][mode] { - break - } + if comment == "" { + comment = nickname } - - if hasPrivs { - if comment == "" { - comment = nickname - } - channel.kickNoMutex(client, target, comment) - } else { - client.Send(nil, client.server.name, ERR_CHANOPRIVSNEEDED, chname, "You're not a channel operator") - } - - channel.membersMutex.Unlock() + channel.Kick(client, target, comment) } return false } @@ -1837,17 +1799,14 @@ type elistMatcher struct { // Matches checks whether the given channel matches our matches. func (matcher *elistMatcher) Matches(channel *Channel) bool { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - if matcher.MinClientsActive { - if len(channel.members) < matcher.MinClients { + if len(channel.Members()) < matcher.MinClients { return false } } if matcher.MaxClientsActive { - if matcher.MaxClients < len(channel.members) { + if len(channel.Members()) < len(channel.members) { return false } } @@ -1933,16 +1892,13 @@ func listHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { // RplList returns the RPL_LIST numeric for the given channel. func (target *Client) RplList(channel *Channel) { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - // get the correct number of channel members var memberCount int - if target.flags[Operator] || channel.members.Has(target) { - memberCount = len(channel.members) + if target.flags[Operator] || channel.hasClient(target) { + memberCount = len(channel.Members()) } else { - for member := range channel.members { - if !member.flags[Invisible] { + for _, member := range channel.Members() { + if !member.HasMode(Invisible) { memberCount++ } } diff --git a/irc/types.go b/irc/types.go index c9322a92..c7474177 100644 --- a/irc/types.go +++ b/irc/types.go @@ -139,13 +139,3 @@ func (members MemberSet) AnyHasMode(mode Mode) bool { // ChannelSet is a set of channels. type ChannelSet map[*Channel]bool - -// Add adds the given channel to this set. -func (channels ChannelSet) Add(channel *Channel) { - channels[channel] = true -} - -// Remove removes the given channel from this set. -func (channels ChannelSet) Remove(channel *Channel) { - delete(channels, channel) -}