refactor idle timeouts

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
+// can receive messages from it).
-// keepalive PING.
 func (client *Client) Touch() {
-	client.timerMutex.Lock()
+	client.idletimer.Touch()
-	defer client.timerMutex.Unlock()
-
-	if client.quitTimer != nil {
-		client.quitTimer.Stop()
 }
 
-	if client.idleTimer == nil {
+// Ping sends the client a PING message.
-		client.idleTimer = time.AfterFunc(IdleTimeout, client.connectionIdle)
+func (client *Client) Ping() {
-	} else {
-		client.idleTimer.Reset(IdleTimeout)
-	}
-}
-
-// 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()
-
 	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 {
+	if client.idletimer != nil {
-		client.idleTimer.Stop()
+		client.idletimer.Stop()
-	}
-	if client.quitTimer != nil {
-		client.quitTimer.Stop()
 	}
 
 	client.socket.Close()

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 !cmd.leaveClientIdle {
-	if client.registered && !cmd.leaveClientIdle {
 		client.Touch()
 	}
 	exiting := cmd.handler(server, client, msg)

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

irc/idletimer.go

@@ -0,0 +1,153 @@
+// Copyright (c) 2017 Shivaram Lingamneni <slingamn@cs.stanford.edu>
+// 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 ""
+	}
+}

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
 	}