Merge pull request #1607 from slingamn/semaphore_update

simplify semaphore release code

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
+		createdTime:     time.Now().UTC(), // may be overwritten by applyRegInfo
-		members:        make(MemberSet),
+		members:         make(MemberSet),
-		name:           name,
+		name:            name,
-		nameCasefolded: casefoldedName,
+		nameCasefolded:  casefoldedName,
-		server:         s,
+		server:          s,
+		writerSemaphore: utils.NewSemaphore(1),
 	}
 
 	channel.initializeLists()
-	channel.writerSemaphore.Initialize(1)
 	channel.history.Initialize(0, 0)
 
 	if !registered {

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,
+		server:          server,
-		accountName:    "*",
+		accountName:     "*",
-		nick:           "*", // * is used until actual nick is given
+		nick:            "*", // * is used until actual nick is given
-		nickCasefolded: "*",
+		nickCasefolded:  "*",
-		nickMaskString: "*", // * is used until actual nick is given
+		nickMaskString:  "*", // * is used until actual nick is given
-		realIP:         realIP,
+		realIP:          realIP,
-		proxiedIP:      proxiedIP,
+		proxiedIP:       proxiedIP,
-		requireSASL:    requireSASL,
+		requireSASL:     requireSASL,
-		nextSessionID:  1,
+		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)
 

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

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 {

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,
+		conn:            conn,
-		maxSendQBytes: maxSendQBytes,
+		maxSendQBytes:   maxSendQBytes,
+		writerSemaphore: utils.NewSemaphore(1),
 	}
-	result.writerSemaphore.Initialize(1)
 	return &result
 }
 

irc/utils/semaphores.go

@@ -5,8 +5,6 @@ package utils
 
 import (
 	"context"
-	"log"
-	"runtime/debug"
 	"time"
 )
 
@@ -14,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.
+// NewSemaphore creates and initializes a semaphore to a given capacity.
-func (semaphore *Semaphore) Initialize(capacity int) {
+func NewSemaphore(capacity int) Semaphore {
-	*semaphore = make(chan empty, capacity)
+	return make(chan empty, capacity)
 }
 
 // Acquire acquires a semaphore, blocking if necessary.
-func (semaphore *Semaphore) Acquire() {
+func (semaphore Semaphore) Acquire() {
-	(*semaphore) <- empty{}
+	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
@@ -38,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
@@ -57,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
@@ -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
+// Release releases a semaphore.
-// to program spurious releases.)
+func (semaphore Semaphore) Release() {
-func (semaphore *Semaphore) Release() {
+	<-semaphore
-	select {
-	case <-(*semaphore):
-		// good
-	default:
-		// spurious release
-		log.Printf("spurious semaphore release (full to capacity %d)", cap(*semaphore))
-		debug.PrintStack()
-	}
 }

irc/utils/semaphores_test.go

@@ -10,8 +10,7 @@ import (
 
 func TestTryAcquire(t *testing.T) {
 	count := 3
-	var sem Semaphore
+	sem := NewSemaphore(count)
-	sem.Initialize(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 := NewSemaphore(1)
-	sem.Initialize(1)
 
 	assertEqual(sem.TryAcquire(), true, t)