Changeset 209 in code for trunk/upstream.go

Timestamp:

Apr 3, 2020, 2:15:25 PM (5 years ago)

Author:

contact

Message:

Fix writer goroutine races

Any SendMessage call after Close could potentially block forever if the
outgoing channel was filled up. Now the channel is drained before the
writer goroutine exits.

File:

• trunk/upstream.go (modified) (3 diffs)

trunk/upstream.go

@@ -114 +114 @@
 
         go func() {
-                // TODO: any SendMessage call after the connection is closed will
-                // either block or drop
-                for {
-                        var closed bool
-                        select {
-                        case msg := <-outgoing:
-                                if uc.srv.Debug {
-                                        uc.logger.Printf("sent: %v", msg)
-                                }
-                                uc.net.SetWriteDeadline(time.Now().Add(writeTimeout))
-                                if err := uc.irc.WriteMessage(msg); err != nil {
-                                        uc.logger.Printf("failed to write message: %v", err)
-                                        closed = true
-                                }
-                        case <-uc.closed:
-                                closed = true
-                        }
-                        if closed {
+                for msg := range outgoing {
+                        if uc.srv.Debug {
+                                uc.logger.Printf("sent: %v", msg)
+                        }
+                        uc.net.SetWriteDeadline(time.Now().Add(writeTimeout))
+                        if err := uc.irc.WriteMessage(msg); err != nil {
+                                uc.logger.Printf("failed to write message: %v", err)
                                 break
                         }
@@ -139 +128 @@
                 } else {
                         uc.logger.Printf("connection closed")
+                }
+                // Drain the outgoing channel to prevent SendMessage from blocking
+                for range outgoing {
+                        // This space is intentionally left blank
                 }
         }()
@@ -160 +153 @@
         }
         close(uc.closed)
+        close(uc.outgoing)
         return nil
 }