diff --git a/irc/client.go b/irc/client.go index 7f1a5386..237ed1a3 100644 --- a/irc/client.go +++ b/irc/client.go @@ -6,6 +6,7 @@ package irc import ( + "errors" "fmt" "log" "net" @@ -13,6 +14,8 @@ import ( "strconv" "time" + "strings" + "github.com/DanielOaks/girc-go/ircmsg" "github.com/DanielOaks/go-ident" ) @@ -25,6 +28,7 @@ const ( var ( TIMEOUT_STATED_SECONDS = strconv.Itoa(int((IDLE_TIMEOUT + QUIT_TIMEOUT).Seconds())) + ErrNickAlreadySet = errors.New("Nickname is already set") ) // Client is an IRC client. @@ -142,12 +146,14 @@ func (client *Client) run() { client.rawHostname = AddrLookupHostname(client.socket.conn.RemoteAddr()) //TODO(dan): Make this a socketreactor from ircbnc + fmt.Println("START", &client) for { line, err = client.socket.Read() if err != nil { client.Quit("connection closed") break } + fmt.Println(" LINE", &client, strings.TrimSpace(line)) msg, err = ircmsg.ParseLine(line) if err != nil { @@ -157,6 +163,7 @@ func (client *Client) run() { cmd, exists := Commands[msg.Command] if !exists { + fmt.Println(" BADLINE", &client, strings.TrimSpace(line)) if len(msg.Command) > 0 { client.Send(nil, client.server.name, ERR_UNKNOWNCOMMAND, client.nick, msg.Command, "Unknown command") } else { @@ -164,11 +171,15 @@ func (client *Client) run() { } continue } + fmt.Println(" GUDLINE", &client, strings.TrimSpace(line)) isExiting = cmd.Run(client.server, client, msg) + fmt.Println(" CMDRUN", &client, strings.TrimSpace(line)) if isExiting || client.isQuitting { + fmt.Println(" BREAKING", &client, strings.TrimSpace(line)) break } + fmt.Println(" CONTINUE", &client, strings.TrimSpace(line)) } // ensure client connection gets closed @@ -349,18 +360,20 @@ func (client *Client) updateNickMask() { } // SetNickname sets the very first nickname for the client. -func (client *Client) SetNickname(nickname string) { +func (client *Client) SetNickname(nickname string) error { if client.HasNick() { Log.error.Printf("%s nickname already set!", client.nickMaskString) - return + return ErrNickAlreadySet } + client.nick = nickname client.updateNick() client.server.clients.Add(client) + return nil } // ChangeNickname changes the existing nickname of the client. -func (client *Client) ChangeNickname(nickname string) { +func (client *Client) ChangeNickname(nickname string) error { origNickMask := client.nickMaskString client.server.clients.Remove(client) client.server.whoWas.Append(client) @@ -370,6 +383,7 @@ func (client *Client) ChangeNickname(nickname string) { for friend := range client.Friends() { friend.Send(nil, origNickMask, "NICK", nickname) } + return nil } func (client *Client) Quit(message string) { diff --git a/irc/client_lookup_set.go b/irc/client_lookup_set.go index 89260615..e754b7b4 100644 --- a/irc/client_lookup_set.go +++ b/irc/client_lookup_set.go @@ -11,6 +11,10 @@ import ( "regexp" "strings" + "sync" + + "runtime/debug" + "github.com/DanielOaks/girc-go/ircmatch" ) @@ -33,8 +37,25 @@ func ExpandUserHost(userhost string) (expanded string) { return } +type LoggingMutex struct { + mutex sync.Mutex +} + +func (lm *LoggingMutex) Lock() { + lm.mutex.Lock() + fmt.Println("--- locked, stack:") + debug.PrintStack() +} + +func (lm *LoggingMutex) Unlock() { + fmt.Println("--- unlocking") + lm.mutex.Unlock() + fmt.Println("--- unlocked") +} + type ClientLookupSet struct { - ByNick map[string]*Client + ByNickMutex LoggingMutex + ByNick map[string]*Client } func NewClientLookupSet() *ClientLookupSet { @@ -44,34 +65,62 @@ func NewClientLookupSet() *ClientLookupSet { } func (clients *ClientLookupSet) Count() int { - return len(clients.ByNick) + clients.ByNickMutex.Lock() + count := len(clients.ByNick) + clients.ByNickMutex.Unlock() + return count } +//TODO(dan): wouldn't it be best to always use Get rather than this? func (clients *ClientLookupSet) Has(nick string) bool { casefoldedName, err := CasefoldName(nick) if err == nil { return false } + clients.ByNickMutex.Lock() _, exists := clients.ByNick[casefoldedName] + clients.ByNickMutex.Unlock() return exists } +func (clients *ClientLookupSet) getNoMutex(nick string) *Client { + casefoldedName, err := CasefoldName(nick) + if err == nil { + cli := clients.ByNick[casefoldedName] + return cli + } + return nil +} + func (clients *ClientLookupSet) Get(nick string) *Client { casefoldedName, err := CasefoldName(nick) if err == nil { - return clients.ByNick[casefoldedName] + clients.ByNickMutex.Lock() + cli := clients.ByNick[casefoldedName] + clients.ByNickMutex.Unlock() + return cli } return nil } func (clients *ClientLookupSet) Add(client *Client) error { + fmt.Println("Initial nick:", client.nick) if !client.HasNick() { return ErrNickMissing } - if clients.Get(client.nick) != nil { + fmt.Println("- getting lock:", client.nick) + clients.ByNickMutex.Lock() + fmt.Println("- got lock:", client.nick) + if clients.getNoMutex(client.nick) != nil { + fmt.Println("- already exists:", client.nick) + clients.ByNickMutex.Unlock() return ErrNicknameInUse } + fmt.Println("- adding:", client.nick) clients.ByNick[client.nickCasefolded] = client + fmt.Println("- set:", client.nick) + clients.ByNickMutex.Unlock() + fmt.Println("- released lock:", client.nick) return nil } @@ -79,16 +128,19 @@ func (clients *ClientLookupSet) Remove(client *Client) error { if !client.HasNick() { return ErrNickMissing } - if clients.Get(client.nick) != client { + if clients.getNoMutex(client.nick) != client { return ErrNicknameMismatch } + clients.ByNickMutex.Lock() delete(clients.ByNick, client.nickCasefolded) + clients.ByNickMutex.Unlock() return nil } func (clients *ClientLookupSet) AllWithCaps(caps ...Capability) (set ClientSet) { set = make(ClientSet) + clients.ByNickMutex.Lock() var client *Client for _, client = range clients.ByNick { // make sure they have all the required caps @@ -100,6 +152,7 @@ func (clients *ClientLookupSet) AllWithCaps(caps ...Capability) (set ClientSet) set.Add(client) } + clients.ByNickMutex.Unlock() return set } @@ -113,11 +166,13 @@ func (clients *ClientLookupSet) FindAll(userhost string) (set ClientSet) { } matcher := ircmatch.MakeMatch(userhost) + clients.ByNickMutex.Lock() for _, client := range clients.ByNick { if matcher.Match(client.nickMaskCasefolded) { set.Add(client) } } + clients.ByNickMutex.Unlock() return set } @@ -128,14 +183,18 @@ func (clients *ClientLookupSet) Find(userhost string) *Client { return nil } matcher := ircmatch.MakeMatch(userhost) + var matchedClient *Client + clients.ByNickMutex.Lock() for _, client := range clients.ByNick { if matcher.Match(client.nickMaskCasefolded) { - return client + matchedClient = client + break } } + clients.ByNickMutex.Unlock() - return nil + return matchedClient } // diff --git a/irc/nickname.go b/irc/nickname.go index 6a34b9ef..4c26146a 100644 --- a/irc/nickname.go +++ b/irc/nickname.go @@ -34,18 +34,23 @@ func nickHandler(server *Server, client *Client, msg ircmsg.IrcMessage) bool { return false } - //TODO(dan): There's probably some races here, we should be changing this in the primary server thread + // bleh, this will be replaced and done below target := server.clients.Get(nickname) if target != nil && target != client { client.Send(nil, server.name, ERR_NICKNAMEINUSE, client.nick, nicknameRaw, "Nickname is already in use") return false } - if client.registered { - client.ChangeNickname(nicknameRaw) - client.alertMonitors() + err = client.ChangeNickname(nicknameRaw) } else { - client.SetNickname(nicknameRaw) + err = client.SetNickname(nicknameRaw) + } + if err != nil { + client.Send(nil, server.name, ERR_NICKNAMEINUSE, client.nick, nicknameRaw, "Nickname is already in use") + return false + } + if client.registered { + client.alertMonitors() } server.tryRegister(client) return false diff --git a/irc/server.go b/irc/server.go index b6605684..dfc2a1d6 100644 --- a/irc/server.go +++ b/irc/server.go @@ -343,9 +343,11 @@ func loadChannelList(channel *Channel, list string, maskMode ChannelMode) { func (server *Server) Shutdown() { //TODO(dan): Make sure we disallow new nicks + server.clients.ByNickMutex.Lock() for _, client := range server.clients.ByNick { client.Notice("Server is shutting down") } + server.clients.ByNickMutex.Unlock() if err := server.store.Close(); err != nil { Log.error.Println("Server.Shutdown store.Close: error:", err) @@ -565,6 +567,7 @@ func (server *Server) wslisten(addr string, tlsMap map[string]*TLSListenConfig) func (server *Server) tryRegister(c *Client) { if c.registered || !c.HasNick() || !c.HasUsername() || (c.capState == CapNegotiating) { + fmt.Println("Try Reg:", &c, c.registered, c.HasNick(), c.HasUsername(), c.capState == CapNegotiating, c.capState) return } c.Register() @@ -1075,12 +1078,14 @@ func (server *Server) rehash() error { server.connectionLimitsMutex.Lock() server.connectionLimits = connectionLimits + server.clients.ByNickMutex.Lock() for _, client := range server.clients.ByNick { ipaddr := net.ParseIP(IPString(client.socket.conn.RemoteAddr())) if ipaddr != nil { server.connectionLimits.AddClient(ipaddr, true) } } + server.clients.ByNickMutex.Unlock() server.connectionLimitsMutex.Unlock() // setup new and removed caps @@ -1147,12 +1152,14 @@ func (server *Server) rehash() error { newISupportReplies := oldISupportList.GetDifference(server.isupport) // push new info to all of our clients + server.clients.ByNickMutex.Lock() for _, sClient := range server.clients.ByNick { for _, tokenline := range newISupportReplies { // ugly trickery ahead sClient.Send(nil, server.name, RPL_ISUPPORT, append([]string{sClient.nick}, tokenline...)...) } } + server.clients.ByNickMutex.Unlock() // destroy old listeners tlsListeners := config.TLSListeners()