From e540fde816fc482bf9f1c914efba3c601b99ecb7 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Sun, 15 Oct 2017 12:24:28 -0400 Subject: [PATCH] refactor idle timeouts --- irc/client.go | 75 +++++++---------------- irc/commands.go | 3 +- irc/getters.go | 12 ++++ irc/idletimer.go | 153 +++++++++++++++++++++++++++++++++++++++++++++++ irc/server.go | 1 - 5 files changed, 189 insertions(+), 55 deletions(-) create mode 100644 irc/idletimer.go diff --git a/irc/client.go b/irc/client.go index 4a3c1e98..ae2d2758 100644 --- a/irc/client.go +++ b/irc/client.go @@ -25,17 +25,17 @@ import ( ) const ( - // IdleTimeout is how long without traffic before a client's considered idle. + // RegisterTimeout is how long clients have to register before we disconnect them + RegisterTimeout = time.Minute + // IdleTimeout is how long without traffic before a registered client is considered idle. IdleTimeout = time.Minute + time.Second*30 - // QuitTimeout is how long without traffic (after they're considered idle) that clients are killed. + // QuitTimeout is how long without traffic before an idle client is disconnected QuitTimeout = time.Minute // IdentTimeoutSeconds is how many seconds before our ident (username) check times out. IdentTimeoutSeconds = 1.5 ) var ( - // TimeoutStatedSeconds is how many seconds before clients are timed out (IdleTimeout plus QuitTimeout). - TimeoutStatedSeconds = strconv.Itoa(int((IdleTimeout + QuitTimeout).Seconds())) // ErrNickAlreadySet is a weird error that's sent when the server's consistency has been compromised. ErrNickAlreadySet = errors.New("Nickname is already set") ) @@ -58,7 +58,7 @@ type Client struct { hasQuit bool hops int hostname string - idleTimer *time.Timer + idletimer *IdleTimer isDestroyed bool isQuitting bool nick string @@ -68,8 +68,6 @@ type Client struct { operName string proxiedIP string // actual remote IP if using the PROXY protocol quitMessage string - quitMessageSent bool - quitTimer *time.Timer rawHostname string realname string registered bool @@ -79,7 +77,6 @@ type Client struct { server *Server socket *Socket stateMutex sync.RWMutex // generic protection for mutable state - timerMutex sync.Mutex username string vhost string whoisLine string @@ -140,7 +137,6 @@ func NewClient(server *Server, conn net.Conn, isTLS bool) *Client { client.Notice("*** Could not find your username") } } - client.Touch() go client.run() return client @@ -194,6 +190,9 @@ func (client *Client) run() { var line string var msg ircmsg.IrcMessage + client.idletimer = NewIdleTimer(client) + client.idletimer.Start() + // Set the hostname for this client // (may be overridden by a later PROXY command from stunnel) client.rawHostname = utils.AddrLookupHostname(client.socket.conn.RemoteAddr()) @@ -247,44 +246,15 @@ func (client *Client) Active() { } // Touch marks the client as alive (as it it has a connection to us and we -// can receive messages from it), and resets when we'll send the client a -// keepalive PING. +// can receive messages from it). func (client *Client) Touch() { - client.timerMutex.Lock() - defer client.timerMutex.Unlock() - - if client.quitTimer != nil { - client.quitTimer.Stop() - } - - if client.idleTimer == nil { - client.idleTimer = time.AfterFunc(IdleTimeout, client.connectionIdle) - } else { - client.idleTimer.Reset(IdleTimeout) - } + client.idletimer.Touch() } -// connectionIdle is run when the client has not sent us any data for a while, -// sends the client a PING and starts the quit timeout. -func (client *Client) connectionIdle() { - client.timerMutex.Lock() - defer client.timerMutex.Unlock() - +// Ping sends the client a PING message. +func (client *Client) Ping() { client.Send(nil, "", "PING", client.nick) - if client.quitTimer == nil { - client.quitTimer = time.AfterFunc(QuitTimeout, client.connectionTimeout) - } else { - client.quitTimer.Reset(QuitTimeout) - } -} - -// connectionTimeout runs after connectionIdle has been run, if we do not receive a -// ping or any other activity back from the client. When this happens we assume the -// connection has died and remove the client from the network. -func (client *Client) connectionTimeout() { - client.Quit(fmt.Sprintf("Ping timeout: %s seconds", TimeoutStatedSeconds)) - client.isQuitting = true } // @@ -293,12 +263,16 @@ func (client *Client) connectionTimeout() { // Register sets the client details as appropriate when entering the network. func (client *Client) Register() { - if client.registered { + client.stateMutex.Lock() + alreadyRegistered := client.registered + client.registered = true + client.stateMutex.Unlock() + + if alreadyRegistered { return } - client.registered = true - client.Touch() + client.Touch() client.updateNickMask("") client.server.monitorManager.AlertAbout(client, true) } @@ -504,9 +478,9 @@ func (client *Client) RplISupport() { // Quit sends the given quit message to the client (but does not destroy them). func (client *Client) Quit(message string) { client.stateMutex.Lock() - alreadyQuit := client.quitMessageSent + alreadyQuit := client.isQuitting if !alreadyQuit { - client.quitMessageSent = true + client.isQuitting = true client.quitMessage = message } client.stateMutex.Unlock() @@ -567,11 +541,8 @@ func (client *Client) destroy() { client.server.clients.Remove(client) // clean up self - if client.idleTimer != nil { - client.idleTimer.Stop() - } - if client.quitTimer != nil { - client.quitTimer.Stop() + if client.idletimer != nil { + client.idletimer.Stop() } client.socket.Close() diff --git a/irc/commands.go b/irc/commands.go index e2fdc5b6..4ec35c5c 100644 --- a/irc/commands.go +++ b/irc/commands.go @@ -39,8 +39,7 @@ func (cmd *Command) Run(server *Server, client *Client, msg ircmsg.IrcMessage) b if !cmd.leaveClientActive { client.Active() } - // only touch client if they're registered so that unregistered clients timeout appropriately - if client.registered && !cmd.leaveClientIdle { + if !cmd.leaveClientIdle { client.Touch() } exiting := cmd.handler(server, client, msg) diff --git a/irc/getters.go b/irc/getters.go index 254a7119..df6fd405 100644 --- a/irc/getters.go +++ b/irc/getters.go @@ -40,3 +40,15 @@ func (client *Client) getNickCasefolded() string { defer client.stateMutex.RUnlock() return client.nickCasefolded } + +func (client *Client) Registered() bool { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + return client.registered +} + +func (client *Client) Destroyed() bool { + client.stateMutex.RLock() + defer client.stateMutex.RUnlock() + return client.isDestroyed +} diff --git a/irc/idletimer.go b/irc/idletimer.go new file mode 100644 index 00000000..95a7c72e --- /dev/null +++ b/irc/idletimer.go @@ -0,0 +1,153 @@ +// Copyright (c) 2017 Shivaram Lingamneni +// released under the MIT license + +package irc + +import ( + "fmt" + "sync" + "time" +) + +// client idleness state machine + +type TimerState uint + +const ( + TimerUnregistered TimerState = iota // client is unregistered + TimerActive // client is actively sending commands + TimerIdle // client is idle, we sent PING and are waiting for PONG +) + +type IdleTimer struct { + sync.Mutex + + // immutable after construction + registerTimeout time.Duration + idleTimeout time.Duration + quitTimeout time.Duration + + // mutable + client *Client + state TimerState + lastSeen time.Time +} + +// NewIdleTimer sets up a new IdleTimer using constant timeouts. +func NewIdleTimer(client *Client) *IdleTimer { + it := IdleTimer{ + registerTimeout: RegisterTimeout, + idleTimeout: IdleTimeout, + quitTimeout: QuitTimeout, + client: client, + state: TimerUnregistered, + } + return &it +} + +// Start starts counting idle time; if there is no activity from the client, +// it will eventually be stopped. +func (it *IdleTimer) Start() { + it.Lock() + it.lastSeen = time.Now() + it.Unlock() + go it.mainLoop() +} + +func (it *IdleTimer) mainLoop() { + for { + it.Lock() + client := it.client + state := it.state + lastSeen := it.lastSeen + it.Unlock() + + if client == nil { + return + } + + registered := client.Registered() + now := time.Now() + idleTime := now.Sub(lastSeen) + newState := state + + switch state { + case TimerUnregistered: + if registered { + // transition to TimerActive state + newState = TimerActive + } + case TimerActive: + if idleTime >= IdleTimeout { + newState = TimerIdle + client.Ping() + } + case TimerIdle: + if idleTime < IdleTimeout { + // new ping came in after we transitioned to TimerIdle + newState = TimerActive + } + } + + it.Lock() + it.state = newState + it.Unlock() + + var nextSleep time.Duration + switch newState { + case TimerUnregistered: + nextSleep = it.registerTimeout - idleTime + case TimerActive: + nextSleep = it.idleTimeout - idleTime + case TimerIdle: + nextSleep = (it.idleTimeout + it.quitTimeout) - idleTime + } + + if nextSleep <= 0 { + // ran out of time, hang them up + client.Quit(it.quitMessage(newState)) + client.destroy() + return + } + + time.Sleep(nextSleep) + } +} + +// Touch registers activity (e.g., sending a command) from an client. +func (it *IdleTimer) Touch() { + it.Lock() + client := it.client + it.Unlock() + + // ignore touches for unregistered clients + if client != nil && !client.Registered() { + return + } + + it.Lock() + it.lastSeen = time.Now() + it.Unlock() +} + +// Stop stops counting idle time. +func (it *IdleTimer) Stop() { + it.Lock() + defer it.Unlock() + // no need to stop the goroutine, it'll clean itself up in a few minutes; + // just ensure the Client object is collectable + it.client = nil +} + +func (it *IdleTimer) quitMessage(state TimerState) string { + switch state { + case TimerUnregistered: + return fmt.Sprintf("Registration timeout: %v", it.registerTimeout) + case TimerIdle: + // how many seconds before registered clients are timed out (IdleTimeout plus QuitTimeout). + return fmt.Sprintf("Ping timeout: %v", (it.idleTimeout + it.quitTimeout)) + default: + // shouldn't happen + return "" + } +} diff --git a/irc/server.go b/irc/server.go index d4eff747..8ebec841 100644 --- a/irc/server.go +++ b/irc/server.go @@ -416,7 +416,6 @@ func (server *Server) tryRegister(c *Client) { reason += fmt.Sprintf(" [%s]", info.Time.Duration.String()) } c.Send(nil, "", "ERROR", fmt.Sprintf("You are banned from this server (%s)", reason)) - c.quitMessageSent = true c.destroy() return }