Changeset 175 in code

Timestamp:

Mar 27, 2020, 10:08:35 PM (5 years ago)

Author:

contact

Message:

Fix race condition in upstreamConn.Close

upstreamConn.closed was a bool accessed from different goroutines. Use
the same pattern as downstreamConn instead.

Location:

trunk

Files:

• upstream.go (modified) (4 diffs)
• user.go (modified) (3 diffs)

trunk/upstream.go

@@ -37 +37 @@
         user     *user
         outgoing chan<- *irc.Message
+        closed   chan struct{}
 
         serverName            string
@@ -48 +49 @@
         username   string
         realname   string
-        closed     bool
         modes      userModes
         channels   map[string]*upstreamChannel
@@ -96 +96 @@
 
         go func() {
-                for msg := range outgoing {
-                        if uc.srv.Debug {
-                                uc.logger.Printf("sent: %v", msg)
-                        }
-                        if err := uc.irc.WriteMessage(msg); err != nil {
-                                uc.logger.Printf("failed to write message: %v", err)
+                for {
+                        var closed bool
+                        select {
+                        case msg := <-outgoing:
+                                if uc.srv.Debug {
+                                        uc.logger.Printf("sent: %v", msg)
+                                }
+                                if err := uc.irc.WriteMessage(msg); err != nil {
+                                        uc.logger.Printf("failed to write message: %v", err)
+                                }
+                        case <-uc.closed:
+                                closed = true
+                        }
+                        if closed {
+                                break
                         }
                 }
@@ -114 +123 @@
 }
 
+func (uc *upstreamConn) isClosed() bool {
+        select {
+        case <-uc.closed:
+                return true
+        default:
+                return false
+        }
+}
+
 func (uc *upstreamConn) Close() error {
-        if uc.closed {
+        if uc.isClosed() {
                 return fmt.Errorf("upstream connection already closed")
         }
-        close(uc.outgoing)
-        uc.closed = true
+        close(uc.closed)
         return nil
 }

trunk/user.go

@@ -65 +65 @@
                 uc.register()
 
+                // TODO: wait for the connection to be registered before adding it to
+                // net, otherwise messages might be sent to it while still being
+                // unauthenticated
                 net.lock.Lock()
                 net.conn = uc
@@ -113 +116 @@
         for _, network := range u.networks {
                 uc := network.upstream()
-                if uc == nil || !uc.registered || uc.closed {
+                if uc == nil || !uc.registered {
                         continue
                 }
@@ -153 +156 @@
                 case eventUpstreamMessage:
                         msg, uc := e.msg, e.uc
-                        if uc.closed {
+                        if uc.isClosed() {
                                 uc.logger.Printf("ignoring message on closed connection: %v", msg)
                                 break