Merge remote-tracking branch 'slingmann/membersmutex.2'

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) regenerateMembersCache() {
-func (channel *Channel) IsEmpty() bool {
+	// this is eventually consistent even without holding the writable Lock()
-	channel.membersMutex.RLock()
+	// throughout the update; all updates to `members` while holding Lock()
-	defer channel.membersMutex.RUnlock()
+	// 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()
+	currentNicks := channel.nicks(client)
-	defer channel.membersMutex.RUnlock()
-
-	channel.namesNoMutex(client)
-}
-
-func (channel *Channel) namesNoMutex(client *Client) {
-	currentNicks := channel.nicksNoMutex(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()
+	channel.stateMutex.RLock()
-	defer channel.membersMutex.RUnlock()
+	defer channel.stateMutex.RUnlock()
-
-	return channel.clientIsAtLeastNoMutex(client, permission)
-}
-
-func (channel *Channel) clientIsAtLeastNoMutex(client *Client, permission Mode) bool {
-	// requires RLock()
 
 	// 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
 }
 
 // <mode> <mode params>
-func (channel *Channel) modeStringNoLock(client *Client) (str string) {
+func (channel *Channel) modeStrings(client *Client) (result []string) {
-	// RLock()
+	isMember := client.HasMode(Operator) || channel.hasClient(client)
-	isMember := client.flags[Operator] || channel.members.Has(client)
-	// RUnlock()
 	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()
+	if channel.hasClient(client) {
-	defer channel.membersMutex.Unlock()
-	if channel.members.Has(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()
+	if !channel.hasClient(client) {
-	defer channel.membersMutex.Unlock()
-
-	if !channel.members.Has(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.
+// SendTopic sends the channel topic to the given client.
-func (channel *Channel) GetTopic(client *Client) {
+func (channel *Channel) SendTopic(client *Client) {
-	channel.membersMutex.RLock()
+	if !channel.hasClient(client) {
-	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) {
 		client.Send(nil, client.server.name, ERR_NOTONCHANNEL, client.nick, channel.name, "You're not on that channel")
 		return
 	}
 
-	if channel.topic == "" {
+	channel.stateMutex.RLock()
-		client.Send(nil, client.server.name, RPL_NOTOPIC, client.nick, channel.name, "No topic is set")
+	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_TOPIC, client.nick, name, 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_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()
+	if !(client.flags[Operator] || channel.hasClient(client)) {
-	defer channel.membersMutex.RUnlock()
-
-	if !(client.flags[Operator] || channel.members.Has(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 {
+	for _, member := range channel.Members() {
-		member.Send(nil, client.nickMaskString, "TOPIC", channel.name, channel.topic)
+		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()
+	channel.stateMutex.RLock()
-	defer channel.membersMutex.RUnlock()
+	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
-	}
+	} else if already {
-
+		return nil
-	switch op {
+	} else {
-	case Add:
-		if channel.members[target][mode] {
-			return nil
-		}
-		channel.members[target][mode] = true
 		return &ModeChange{
-			op:   Add,
+			op:   op,
-			mode: mode,
-			arg:  nick,
-		}
-
-	case Remove:
-		if !channel.members[target][mode] {
-			return nil
-		}
-		channel.members[target][mode] = false
-		return &ModeChange{
-			op:   Remove,
 			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.
+// Quit removes the given client from the channel
-func (channel *Channel) Quit(client *Client, friends *ClientSet) {
+func (channel *Channel) Quit(client *Client) {
-	channel.membersMutex.Lock()
+	channel.stateMutex.Lock()
-	defer channel.membersMutex.Unlock()
-
-	channel.quitNoMutex(client)
-
-	for friend := range channel.members {
-		friends.Add(friend)
-	}
-}
-
-func (channel *Channel) quitNoMutex(client *Client) {
 	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) {
+func (channel *Channel) Kick(client *Client, target *Client, comment string) {
-	// needs a Lock()
+	if !(client.flags[Operator] || channel.hasClient(client)) {
-
-	if !(client.flags[Operator] || channel.members.Has(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 !channel.ClientHasPrivsOver(client, target) {
-	if len(comment) > client.server.limits.KickLen {
+		client.Send(nil, client.server.name, ERR_CHANOPRIVSNEEDED, channel.name, "You're not a channel operator")
-		comment = comment[:client.server.limits.KickLen]
+		return
 	}
 
-	for member := range channel.members {
+	kicklimit := client.server.getLimits().KickLen
-		member.Send(nil, client.nickMaskString, "KICK", channel.name, target.nick, comment)
+	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()
+	if !channel.hasClient(inviter) {
-	defer channel.membersMutex.RUnlock()
-
-	if !channel.members.Has(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)
 		}
 	}
 

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...)
 				}
 			}

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 {
+	for _, channel := range client.Channels() {
-		channel.membersMutex.RLock()
+		for _, member := range channel.Members() {
-		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()
+}

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()
+	set.Unlock()
-	return true
+
+	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] {
+	set.Lock()
-		return false
+	already := !set.masks[mask]
-	}
 	delete(set.masks, mask)
-	set.setRegexp()
+	set.Unlock()
-	return true
+
+	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 {
+	var re *regexp.Regexp
-		set.regexp = nil
-		return
-	}
 
+	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.RUnlock()
-	set.regexp, _ = regexp.Compile(expr)
+
+	if index > 0 {
+		expr := "^" + strings.Join(maskExprs, "|") + "$"
+		re, _ = regexp.Compile(expr)
+	}
+
+	set.Lock()
+	set.regexp = re
+	set.Unlock()
 }

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
+}

irc/modes.go

@@ -327,9 +327,7 @@ func (client *Client) applyUserModeChanges(force bool, changes ModeChanges) Mode
 // MODE <target> [<modestring> [<mode arguments>...]]
 func umodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 	nickname, err := CasefoldName(msg.Params[0])
-
 	target := server.clients.Get(nickname)
-
 	if err != nil || target == nil {
 		if len(msg.Params[0]) > 0 {
 			client.Send(nil, server.name, ERR_NOSUCHNICK, client.nick, msg.Params[0], "No such nick")
@@ -337,7 +335,10 @@ func umodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 		return false
 	}
 
-	if client != target && msg.Command != "SAMODE" {
+	targetNick := target.getNick()
+	hasPrivs := client == target || msg.Command == "SAMODE"
+
+	if !hasPrivs {
 		if len(msg.Params) > 1 {
 			client.Send(nil, server.name, ERR_USERSDONTMATCH, client.nick, "Can't change modes for other users")
 		} else {
@@ -367,13 +368,13 @@ func umodeHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool {
 	}
 
 	if len(applied) > 0 {
-		client.Send(nil, client.nickMaskString, "MODE", target.nick, applied.String())
+		client.Send(nil, client.nickMaskString, "MODE", targetNick, applied.String())
-	} else if client == target {
+	} else if hasPrivs {
-		client.Send(nil, target.nickMaskString, RPL_UMODEIS, target.nick, target.ModeString())
+		client.Send(nil, target.nickMaskString, RPL_UMODEIS, targetNick, target.ModeString())
 		if client.flags[LocalOperator] || client.flags[Operator] {
 			masks := server.snomasks.String(client)
 			if 0 < len(masks) {
-				client.Send(nil, target.nickMaskString, RPL_SNOMASKIS, target.nick, masks, "Server notice masks")
+				client.Send(nil, target.nickMaskString, RPL_SNOMASKIS, targetNick, masks, "Server notice masks")
 			}
 		}
 	}
@@ -475,11 +476,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 +499,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 +513,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 +534,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 {
+			if change.op == List {
-			case Add:
+				continue
-				if channel.flags[change.mode] {
+			}
-					continue
-				}
-				channel.flags[change.mode] = true
-				applied = append(applied, change)
 
-			case Remove:
+			already := channel.setMode(change.mode, change.op == Add)
-				if !channel.flags[change.mode] {
+			if !already {
-					continue
-				}
-				delete(channel.flags, change.mode)
 				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 {
+			// Admins can't give other people Admin or remove it from others,
-				for _, mode := range ChannelPrivModes {
+			// standard for that channel mode, we worry about this later
-					if channel.members[client][mode] {
+			if !hasPrivs && change.mode != ChannelAdmin {
-						hasPrivs = true
+				hasPrivs = channel.ClientIsAtLeast(client, change.mode)
-
-						// 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
-					}
-				}
 			}
 
 			casefoldedName, err := CasefoldName(change.arg)
@@ -634,9 +613,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 +630,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 +683,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}, channel.modeStrings(client)...)
-		args := append([]string{client.nick, channel.name}, strings.Split(channel.modeStringNoLock(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))
 	}

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)

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() {
-	for channel := range client.channels {
-		channel.membersMutex.RLock()
-		defer channel.membersMutex.RUnlock()
-
 		// 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)
+		chstrs = append(chstrs, channel.ClientPrefixes(client, isMultiPrefix)+channel.name)
-		index++
 	}
 	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.
 // <channel> <user> <host> <server> <nick> ( "H" / "G" ) ["*"] [ ( "@" / "+" ) ]
 // :<hopcount> <real name>
-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()
+	for _, member := range channel.Members() {
-	defer channel.membersMutex.RUnlock()
-
-	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
+		if comment == "" {
-		//TODO(dan): split this into a separate function that checks if users have privs
+			comment = nickname
-		// 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
-			}
 		}
-
+		channel.Kick(client, target, comment)
-		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()
 	}
 	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) {
+	if target.flags[Operator] || channel.hasClient(target) {
-		memberCount = len(channel.members)
+		memberCount = len(channel.Members())
 	} else {
-		for member := range channel.members {
+		for _, member := range channel.Members() {
-			if !member.flags[Invisible] {
+			if !member.HasMode(Invisible) {
 				memberCount++
 			}
 		}

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)
-}