From 549d06bc984643555a0831e815a4e357593f1940 Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 7 Apr 2021 08:33:19 -0400 Subject: [PATCH 1/2] simplify semaphore release code --- irc/utils/semaphores.go | 14 ++------------ 1 file changed, 2 insertions(+), 12 deletions(-) diff --git a/irc/utils/semaphores.go b/irc/utils/semaphores.go index 2268cde7..431baee6 100644 --- a/irc/utils/semaphores.go +++ b/irc/utils/semaphores.go @@ -5,8 +5,6 @@ package utils import ( "context" - "log" - "runtime/debug" "time" ) @@ -67,15 +65,7 @@ func (semaphore *Semaphore) AcquireWithContext(ctx context.Context) (acquired bo return } -// Release releases a semaphore. It never blocks. (This is not a license -// to program spurious releases.) +// Release releases a semaphore. func (semaphore *Semaphore) Release() { - select { - case <-(*semaphore): - // good - default: - // spurious release - log.Printf("spurious semaphore release (full to capacity %d)", cap(*semaphore)) - debug.PrintStack() - } + <-(*semaphore) } From 5b33cd436f258849a4695e5f7611a9de5638144b Mon Sep 17 00:00:00 2001 From: Shivaram Lingamneni Date: Wed, 7 Apr 2021 08:44:17 -0400 Subject: [PATCH 2/2] remove unnecessary indirection in semaphore --- irc/channel.go | 12 ++++++------ irc/client.go | 23 ++++++++++++----------- irc/semaphores.go | 2 +- irc/server.go | 4 ++-- irc/socket.go | 6 +++--- irc/utils/semaphores.go | 26 +++++++++++++------------- irc/utils/semaphores_test.go | 6 ++---- 7 files changed, 39 insertions(+), 40 deletions(-) diff --git a/irc/channel.go b/irc/channel.go index 4b341e3e..c3c766b2 100644 --- a/irc/channel.go +++ b/irc/channel.go @@ -61,15 +61,15 @@ func NewChannel(s *Server, name, casefoldedName string, registered bool) *Channe config := s.Config() channel := &Channel{ - createdTime: time.Now().UTC(), // may be overwritten by applyRegInfo - members: make(MemberSet), - name: name, - nameCasefolded: casefoldedName, - server: s, + createdTime: time.Now().UTC(), // may be overwritten by applyRegInfo + members: make(MemberSet), + name: name, + nameCasefolded: casefoldedName, + server: s, + writerSemaphore: utils.NewSemaphore(1), } channel.initializeLists() - channel.writerSemaphore.Initialize(1) channel.history.Initialize(0, 0) if !registered { diff --git a/irc/client.go b/irc/client.go index c7e36b5c..f34a3de8 100644 --- a/irc/client.go +++ b/irc/client.go @@ -356,20 +356,20 @@ func (server *Server) RunClient(conn IRCConn) { Duration: config.Accounts.LoginThrottling.Duration, Limit: config.Accounts.LoginThrottling.MaxAttempts, }, - server: server, - accountName: "*", - nick: "*", // * is used until actual nick is given - nickCasefolded: "*", - nickMaskString: "*", // * is used until actual nick is given - realIP: realIP, - proxiedIP: proxiedIP, - requireSASL: requireSASL, - nextSessionID: 1, + server: server, + accountName: "*", + nick: "*", // * is used until actual nick is given + nickCasefolded: "*", + nickMaskString: "*", // * is used until actual nick is given + realIP: realIP, + proxiedIP: proxiedIP, + requireSASL: requireSASL, + nextSessionID: 1, + writerSemaphore: utils.NewSemaphore(1), } if requireSASL { client.requireSASLMessage = banMsg } - client.writerSemaphore.Initialize(1) client.history.Initialize(config.History.ClientLength, time.Duration(config.History.AutoresizeWindow)) client.brbTimer.Initialize(client) session := &Session{ @@ -445,6 +445,8 @@ func (server *Server) AddAlwaysOnClient(account ClientAccount, channelToStatus m realname: realname, nextSessionID: 1, + + writerSemaphore: utils.NewSemaphore(1), } if client.checkAlwaysOnExpirationNoMutex(config, true) { @@ -456,7 +458,6 @@ func (server *Server) AddAlwaysOnClient(account ClientAccount, channelToStatus m for _, m := range uModes { client.SetMode(m, true) } - client.writerSemaphore.Initialize(1) client.history.Initialize(0, 0) client.brbTimer.Initialize(client) diff --git a/irc/semaphores.go b/irc/semaphores.go index 2aa5a165..1847c87a 100644 --- a/irc/semaphores.go +++ b/irc/semaphores.go @@ -37,5 +37,5 @@ func (serversem *ServerSemaphores) Initialize() { if capacity > MaxServerSemaphoreCapacity { capacity = MaxServerSemaphoreCapacity } - serversem.ClientDestroy.Initialize(capacity) + serversem.ClientDestroy = utils.NewSemaphore(capacity) } diff --git a/irc/server.go b/irc/server.go index 1fece0c1..6dd42b64 100644 --- a/irc/server.go +++ b/irc/server.go @@ -612,11 +612,11 @@ func (server *Server) applyConfig(config *Config) (err error) { if initial { maxIPConc := int(config.Server.IPCheckScript.MaxConcurrency) if maxIPConc != 0 { - server.semaphores.IPCheckScript.Initialize(maxIPConc) + server.semaphores.IPCheckScript = utils.NewSemaphore(maxIPConc) } maxAuthConc := int(config.Accounts.AuthScript.MaxConcurrency) if maxAuthConc != 0 { - server.semaphores.AuthScript.Initialize(maxAuthConc) + server.semaphores.AuthScript = utils.NewSemaphore(maxAuthConc) } if err := overrideServicePrefixes(config.Server.OverrideServicesHostname); err != nil { diff --git a/irc/socket.go b/irc/socket.go index c4cc3e45..ff5ac5c1 100644 --- a/irc/socket.go +++ b/irc/socket.go @@ -40,10 +40,10 @@ type Socket struct { // NewSocket returns a new Socket. func NewSocket(conn IRCConn, maxSendQBytes int) *Socket { result := Socket{ - conn: conn, - maxSendQBytes: maxSendQBytes, + conn: conn, + maxSendQBytes: maxSendQBytes, + writerSemaphore: utils.NewSemaphore(1), } - result.writerSemaphore.Initialize(1) return &result } diff --git a/irc/utils/semaphores.go b/irc/utils/semaphores.go index 431baee6..564d1b34 100644 --- a/irc/utils/semaphores.go +++ b/irc/utils/semaphores.go @@ -12,21 +12,21 @@ import ( // A semaphore of capacity 1 can be used as a trylock. type Semaphore (chan empty) -// Initialize initializes a semaphore to a given capacity. -func (semaphore *Semaphore) Initialize(capacity int) { - *semaphore = make(chan empty, capacity) +// NewSemaphore creates and initializes a semaphore to a given capacity. +func NewSemaphore(capacity int) Semaphore { + return make(chan empty, capacity) } // Acquire acquires a semaphore, blocking if necessary. -func (semaphore *Semaphore) Acquire() { - (*semaphore) <- empty{} +func (semaphore Semaphore) Acquire() { + semaphore <- empty{} } // TryAcquire tries to acquire a semaphore, returning whether the acquire was // successful. It never blocks. -func (semaphore *Semaphore) TryAcquire() (acquired bool) { +func (semaphore Semaphore) TryAcquire() (acquired bool) { select { - case (*semaphore) <- empty{}: + case semaphore <- empty{}: return true default: return false @@ -36,14 +36,14 @@ func (semaphore *Semaphore) TryAcquire() (acquired bool) { // AcquireWithTimeout tries to acquire a semaphore, blocking for a maximum // of approximately `d` while waiting for it. It returns whether the acquire // was successful. -func (semaphore *Semaphore) AcquireWithTimeout(timeout time.Duration) (acquired bool) { +func (semaphore Semaphore) AcquireWithTimeout(timeout time.Duration) (acquired bool) { if timeout < 0 { return semaphore.TryAcquire() } timer := time.NewTimer(timeout) select { - case (*semaphore) <- empty{}: + case semaphore <- empty{}: acquired = true case <-timer.C: acquired = false @@ -55,9 +55,9 @@ func (semaphore *Semaphore) AcquireWithTimeout(timeout time.Duration) (acquired // AcquireWithContext tries to acquire a semaphore, blocking at most until // the context expires. It returns whether the acquire was successful. // Note that if the context is already expired, the acquire may succeed anyway. -func (semaphore *Semaphore) AcquireWithContext(ctx context.Context) (acquired bool) { +func (semaphore Semaphore) AcquireWithContext(ctx context.Context) (acquired bool) { select { - case (*semaphore) <- empty{}: + case semaphore <- empty{}: acquired = true case <-ctx.Done(): acquired = false @@ -66,6 +66,6 @@ func (semaphore *Semaphore) AcquireWithContext(ctx context.Context) (acquired bo } // Release releases a semaphore. -func (semaphore *Semaphore) Release() { - <-(*semaphore) +func (semaphore Semaphore) Release() { + <-semaphore } diff --git a/irc/utils/semaphores_test.go b/irc/utils/semaphores_test.go index b047ed56..8dbb5502 100644 --- a/irc/utils/semaphores_test.go +++ b/irc/utils/semaphores_test.go @@ -10,8 +10,7 @@ import ( func TestTryAcquire(t *testing.T) { count := 3 - var sem Semaphore - sem.Initialize(count) + sem := NewSemaphore(count) for i := 0; i < count; i++ { assertEqual(sem.TryAcquire(), true, t) @@ -24,8 +23,7 @@ func TestTryAcquire(t *testing.T) { } func TestAcquireWithTimeout(t *testing.T) { - var sem Semaphore - sem.Initialize(1) + sem := NewSemaphore(1) assertEqual(sem.TryAcquire(), true, t)