From c39bebc696b5a204dde8740f0134bed296d80090 Mon Sep 17 00:00:00 2001 From: Daniel Oaks Date: Wed, 11 Jan 2017 12:16:51 +1000 Subject: [PATCH] Fix various locks around joining, kicking and quitting --- irc/channel.go | 53 ++++++++++++++++++++++++++++++++++++-------------- irc/server.go | 7 ++++--- 2 files changed, 42 insertions(+), 18 deletions(-) diff --git a/irc/channel.go b/irc/channel.go index a6755d22..8114803c 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -68,11 +68,23 @@ func NewChannel(s *Server, name string, addDefaultModes bool) *Channel { func (channel *Channel) IsEmpty() bool { channel.membersMutex.RLock() defer channel.membersMutex.RUnlock() + + return channel.isEmptyNoMutex() +} + +func (channel *Channel) isEmptyNoMutex() bool { return len(channel.members) == 0 } func (channel *Channel) Names(client *Client) { - currentNicks := channel.Nicks(client) + channel.membersMutex.RLock() + 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 @@ -101,6 +113,12 @@ func (channel *Channel) ClientIsAtLeast(client *Client, permission ChannelMode) channel.membersMutex.RLock() defer channel.membersMutex.RUnlock() + return channel.clientIsAtLeastNoMutex(client, permission) +} + +func (channel *Channel) clientIsAtLeastNoMutex(client *Client, permission ChannelMode) bool { + // requires RLock() + // get voice, since it's not a part of ChannelPrivModes if channel.members.HasMode(client, permission) { return true @@ -141,10 +159,7 @@ func (modes ChannelModeSet) Prefixes(isMultiPrefix bool) string { return prefixes } -func (channel *Channel) Nicks(target *Client) []string { - channel.membersMutex.RLock() - defer channel.membersMutex.RUnlock() - +func (channel *Channel) nicksNoMutex(target *Client) []string { isMultiPrefix := (target != nil) && target.capabilities[MultiPrefix] isUserhostInNames := (target != nil) && target.capabilities[UserhostInNames] nicks := make([]string, len(channel.members)) @@ -218,12 +233,11 @@ func (channel *Channel) CheckKey(key string) bool { func (channel *Channel) Join(client *Client, key string) { channel.membersMutex.Lock() - defer channel.membersMutex.Unlock() - if channel.members.Has(client) { // already joined, no message? return } + channel.membersMutex.Unlock() if channel.IsFull() { client.Send(nil, client.server.name, ERR_CHANNELISFULL, channel.name, "Cannot join channel (+l)") @@ -241,6 +255,8 @@ func (channel *Channel) Join(client *Client, key string) { return } + channel.membersMutex.Lock() + defer channel.membersMutex.Unlock() if channel.lists[BanMask].Match(client.nickMaskCasefolded) && !isInvited && !channel.lists[ExceptMask].Match(client.nickMaskCasefolded) { @@ -270,8 +286,8 @@ func (channel *Channel) Join(client *Client, key string) { } else { client.Send(nil, client.nickMaskString, "JOIN", channel.name) } - channel.GetTopic(client) - channel.Names(client) + channel.getTopicNoMutex(client) // we already have Lock + channel.namesNoMutex(client) } func (channel *Channel) Part(client *Client, message string) { @@ -293,6 +309,10 @@ func (channel *Channel) GetTopic(client *Client) { channel.membersMutex.RLock() defer channel.membersMutex.RUnlock() + channel.getTopicNoMutex(client) +} + +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 @@ -507,23 +527,26 @@ func (channel *Channel) Quit(client *Client) { channel.membersMutex.Lock() defer channel.membersMutex.Unlock() + channel.quitNoMutex(client) +} + +func (channel *Channel) quitNoMutex(client *Client) { channel.members.Remove(client) client.channels.Remove(channel) - if channel.IsEmpty() { + if channel.isEmptyNoMutex() { channel.server.channels.Remove(channel) } } -func (channel *Channel) Kick(client *Client, target *Client, comment string) { - channel.membersMutex.Lock() - defer channel.membersMutex.Unlock() +func (channel *Channel) kickNoMutex(client *Client, target *Client, comment string) { + // needs a Lock() 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.ClientIsAtLeast(client, ChannelOperator) { + if !channel.clientIsAtLeastNoMutex(client, ChannelOperator) { client.Send(nil, client.server.name, ERR_CANNOTSENDTOCHAN, channel.name, "Cannot send to channel") return } @@ -539,7 +562,7 @@ func (channel *Channel) Kick(client *Client, target *Client, comment string) { for member := range channel.members { member.Send(nil, client.nickMaskString, "KICK", channel.name, target.nick, comment) } - channel.Quit(target) + channel.quitNoMutex(target) } func (channel *Channel) Invite(invitee *Client, inviter *Client) { diff --git a/irc/server.go b/irc/server.go index fc1655a0..e617dbee 100644 --- a/irc/server.go +++ b/irc/server.go @@ -1381,8 +1381,7 @@ func kickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { // 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.RLock() - defer channel.membersMutex.RUnlock() + channel.membersMutex.Lock() var hasPrivs bool for _, mode := range ChannelPrivModes { @@ -1404,10 +1403,12 @@ func kickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { if comment == "" { comment = nickname } - channel.Kick(client, target, comment) + 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 }