Merge pull request #443 from slingamn/remove_indirections.4

remove indirections in Fakelag and NickTimer

irc/client.go

@@ -57,7 +57,7 @@ type Client struct {
 	channels           ChannelSet
 	ctime              time.Time
 	exitedSnomaskSent  bool
-	fakelag            *Fakelag
+	fakelag            Fakelag
 	flags              *modes.ModeSet
 	hasQuit            bool
 	hops               int
@@ -75,7 +75,7 @@ type Client struct {
 	nickCasefolded     string
 	nickMaskCasefolded string
 	nickMaskString     string // cache for nickmask string since it's used with lots of replies
-	nickTimer          *NickTimer
+	nickTimer          NickTimer
 	oper               *Oper
 	preregNick         string
 	proxiedIP          net.IP // actual remote IP if using the PROXY protocol
@@ -217,23 +217,9 @@ func (client *Client) isAuthorized(config *Config) bool {
 }
 
 func (client *Client) resetFakelag() {
-	fakelag := func() *Fakelag {
-		if client.HasRoleCapabs("nofakelag") {
-			return nil
-		}
-
-		flc := client.server.FakelagConfig()
-
-		if !flc.Enabled {
-			return nil
-		}
-
-		return NewFakelag(flc.Window, flc.BurstLimit, flc.MessagesPerWindow, flc.Cooldown)
-	}()
-
-	client.stateMutex.Lock()
-	defer client.stateMutex.Unlock()
-	client.fakelag = fakelag
+	var flc FakelagConfig = client.server.Config().Fakelag
+	flc.Enabled = flc.Enabled && !client.HasRoleCapabs("nofakelag")
+	client.fakelag.Initialize(flc)
 }
 
 // IP returns the IP address of this client.
@@ -309,7 +295,7 @@ func (client *Client) run() {
 	client.idletimer = NewIdleTimer(client)
 	client.idletimer.Start()
 
-	client.nickTimer = NewNickTimer(client)
+	client.nickTimer.Initialize(client)
 
 	client.resetFakelag()
 

irc/fakelag.go

@@ -24,33 +24,25 @@ const (
 // this is intentionally not threadsafe, because it should only be touched
 // from the loop that accepts the client's input and runs commands
 type Fakelag struct {
-	window                    time.Duration
-	burstLimit                uint
-	throttleMessagesPerWindow uint
-	cooldown                  time.Duration
-	nowFunc                   func() time.Time
-	sleepFunc                 func(time.Duration)
+	config    FakelagConfig
+	nowFunc   func() time.Time
+	sleepFunc func(time.Duration)
 
 	state      FakelagState
 	burstCount uint // number of messages sent in the current burst
 	lastTouch  time.Time
 }
 
-func NewFakelag(window time.Duration, burstLimit uint, throttleMessagesPerWindow uint, cooldown time.Duration) *Fakelag {
-	return &Fakelag{
-		window:                    window,
-		burstLimit:                burstLimit,
-		throttleMessagesPerWindow: throttleMessagesPerWindow,
-		cooldown:                  cooldown,
-		nowFunc:                   time.Now,
-		sleepFunc:                 time.Sleep,
-		state:                     FakelagBursting,
-	}
+func (fl *Fakelag) Initialize(config FakelagConfig) {
+	fl.config = config
+	fl.nowFunc = time.Now
+	fl.sleepFunc = time.Sleep
+	fl.state = FakelagBursting
 }
 
 // register a new command, sleep if necessary to delay it
 func (fl *Fakelag) Touch() {
-	if fl == nil {
+	if !fl.config.Enabled {
 		return
 	}
 
@@ -61,12 +53,12 @@ func (fl *Fakelag) Touch() {
 
 	if fl.state == FakelagBursting {
 		// determine if the previous burst is over
-		if elapsed > fl.cooldown {
+		if elapsed > fl.config.Cooldown {
 			fl.burstCount = 0
 		}
 
 		fl.burstCount++
-		if fl.burstCount > fl.burstLimit {
+		if fl.burstCount > fl.config.BurstLimit {
 			// reset burst window for next time
 			fl.burstCount = 0
 			// transition to throttling
@@ -78,13 +70,13 @@ func (fl *Fakelag) Touch() {
 	}
 
 	if fl.state == FakelagThrottled {
-		if elapsed > fl.cooldown {
+		if elapsed > fl.config.Cooldown {
 			// let them burst again
 			fl.state = FakelagBursting
 			return
 		}
 		// space them out by at least window/messagesperwindow
-		sleepDuration := time.Duration((int64(fl.window) / int64(fl.throttleMessagesPerWindow)) - int64(elapsed))
+		sleepDuration := time.Duration((int64(fl.config.Window) / int64(fl.config.MessagesPerWindow)) - int64(elapsed))
 		if sleepDuration > 0 {
 			fl.sleepFunc(sleepDuration)
 			// the touch time should take into account the time we slept

irc/fakelag_test.go

@@ -40,13 +40,20 @@ func (mt *mockTime) lastSleep() (slept bool, duration time.Duration) {
 }
 
 func newFakelagForTesting(window time.Duration, burstLimit uint, throttleMessagesPerWindow uint, cooldown time.Duration) (*Fakelag, *mockTime) {
-	fl := NewFakelag(window, burstLimit, throttleMessagesPerWindow, cooldown)
+	fl := Fakelag{}
+	fl.config = FakelagConfig{
+		Enabled:           true,
+		Window:            window,
+		BurstLimit:        burstLimit,
+		MessagesPerWindow: throttleMessagesPerWindow,
+		Cooldown:          cooldown,
+	}
 	mt := new(mockTime)
 	mt.now, _ = time.Parse("Mon Jan 2 15:04:05 -0700 MST 2006", "Mon Jan 2 15:04:05 -0700 MST 2006")
 	mt.lastCheckedSleep = -1
 	fl.nowFunc = mt.Now
 	fl.sleepFunc = mt.Sleep
-	return fl, mt
+	return &fl, mt
 }
 
 func TestFakelag(t *testing.T) {

irc/getters.go

@@ -46,10 +46,6 @@ func (server *Server) AccountConfig() *AccountConfig {
 	return &server.Config().Accounts
 }
 
-func (server *Server) FakelagConfig() *FakelagConfig {
-	return &server.Config().Fakelag
-}
-
 func (server *Server) GetOperator(name string) (oper *Oper) {
 	name, err := CasefoldName(name)
 	if err != nil {

irc/idletimer.go

@@ -6,6 +6,7 @@ package irc
 import (
 	"fmt"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/goshuirc/irc-go/ircfmt"
@@ -180,33 +181,50 @@ type NickTimer struct {
 	sync.Mutex // tier 1
 
 	// immutable after construction
-	timeout time.Duration
-	client  *Client
+	client *Client
 
 	// mutable
-	stopped        bool
 	nick           string
 	accountForNick string
 	account        string
+	timeout        time.Duration
 	timer          *time.Timer
+	enabled        uint32
 }
 
-// NewNickTimer sets up a new nick timer (returning nil if timeout enforcement is not enabled)
-func NewNickTimer(client *Client) *NickTimer {
-	config := client.server.AccountConfig().NickReservation
-	if !(config.Enabled && (config.Method == NickReservationWithTimeout || config.AllowCustomEnforcement)) {
-		return nil
+// Initialize sets up a NickTimer, based on server config settings.
+func (nt *NickTimer) Initialize(client *Client) {
+	if nt.client == nil {
+		nt.client = client // placate the race detector
 	}
 
-	return &NickTimer{
-		client:  client,
-		timeout: config.RenameTimeout,
+	config := &client.server.Config().Accounts.NickReservation
+	enabled := config.Enabled && (config.Method == NickReservationWithTimeout || config.AllowCustomEnforcement)
+
+	nt.Lock()
+	defer nt.Unlock()
+	nt.timeout = config.RenameTimeout
+	if enabled {
+		atomic.StoreUint32(&nt.enabled, 1)
+	} else {
+		nt.stopInternal()
 	}
 }
 
+func (nt *NickTimer) Enabled() bool {
+	return atomic.LoadUint32(&nt.enabled) == 1
+}
+
+func (nt *NickTimer) Timeout() (timeout time.Duration) {
+	nt.Lock()
+	timeout = nt.timeout
+	nt.Unlock()
+	return
+}
+
 // Touch records a nick change and updates the timer as necessary
 func (nt *NickTimer) Touch() {
-	if nt == nil {
+	if !nt.Enabled() {
 		return
 	}
 
@@ -215,16 +233,12 @@ func (nt *NickTimer) Touch() {
 	accountForNick, method := nt.client.server.accounts.EnforcementStatus(cfnick, skeleton)
 	enforceTimeout := method == NickReservationWithTimeout
 
-	var shouldWarn bool
+	var shouldWarn, shouldRename bool
 
 	func() {
 		nt.Lock()
 		defer nt.Unlock()
 
-		if nt.stopped {
-			return
-		}
-
 		// the timer will not reset as long as the squatter is targeting the same account
 		accountChanged := accountForNick != nt.accountForNick
 		// change state
@@ -237,38 +251,39 @@ func (nt *NickTimer) Touch() {
 			nt.timer.Stop()
 			nt.timer = nil
 		}
-		if enforceTimeout && delinquent && accountChanged {
+		if enforceTimeout && delinquent && (accountChanged || nt.timer == nil) {
 			nt.timer = time.AfterFunc(nt.timeout, nt.processTimeout)
 			shouldWarn = true
+		} else if method == NickReservationStrict && delinquent {
+			shouldRename = true // this can happen if reservation was enabled by rehash
 		}
 	}()
 
 	if shouldWarn {
-		nt.sendWarning()
+		nt.client.Send(nil, "NickServ", "NOTICE", nt.client.Nick(), fmt.Sprintf(ircfmt.Unescape(nt.client.t(nsTimeoutNotice)), nt.Timeout()))
+	} else if shouldRename {
+		nt.client.Notice(nt.client.t("Nickname is reserved by a different account"))
+		nt.client.server.RandomlyRename(nt.client)
 	}
 }
 
 // Stop stops counting time and cleans up the timer
 func (nt *NickTimer) Stop() {
-	if nt == nil {
-		return
-	}
-
 	nt.Lock()
 	defer nt.Unlock()
+	nt.stopInternal()
+}
+
+func (nt *NickTimer) stopInternal() {
 	if nt.timer != nil {
 		nt.timer.Stop()
 		nt.timer = nil
 	}
-	nt.stopped = true
-}
-
-func (nt *NickTimer) sendWarning() {
-	nt.client.Send(nil, "NickServ", "NOTICE", nt.client.Nick(), fmt.Sprintf(ircfmt.Unescape(nt.client.t(nsTimeoutNotice)), nt.timeout))
+	atomic.StoreUint32(&nt.enabled, 0)
 }
 
 func (nt *NickTimer) processTimeout() {
 	baseMsg := "Nick is reserved and authentication timeout expired: %v"
-	nt.client.Notice(fmt.Sprintf(nt.client.t(baseMsg), nt.timeout))
+	nt.client.Notice(fmt.Sprintf(nt.client.t(baseMsg), nt.Timeout()))
 	nt.client.server.RandomlyRename(nt.client)
 }

irc/server.go

@@ -826,6 +826,13 @@ func (server *Server) applyConfig(config *Config, initial bool) (err error) {
 			if sendRawOutputNotice {
 				sClient.Notice(sClient.t("This server is in debug mode and is logging all user I/O. If you do not wish for everything you send to be readable by the server owner(s), please disconnect."))
 			}
+
+			if !oldConfig.Accounts.NickReservation.Enabled && config.Accounts.NickReservation.Enabled {
+				sClient.nickTimer.Initialize(sClient)
+				sClient.nickTimer.Touch()
+			} else if oldConfig.Accounts.NickReservation.Enabled && !config.Accounts.NickReservation.Enabled {
+				sClient.nickTimer.Stop()
+			}
 		}
 	}