Changeset 175 in code for trunk


Ignore:
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:
2 edited

Legend:

Unmodified
Added
Removed

  • trunk/upstream.go

    r174 r175  
    3737        user     *user
    3838        outgoing chan<- *irc.Message
     39        closed   chan struct{}
    3940
    4041        serverName            string
    … …  
    4849        username   string
    4950        realname   string
    50         closed     bool
    5151        modes      userModes
    5252        channels   map[string]*upstreamChannel
    … …  
    9696
    9797        go func() {
    98                 for msg := range outgoing {
    99                         if uc.srv.Debug {
    100                                 uc.logger.Printf("sent: %v", msg)
    101                         }
    102                         if err := uc.irc.WriteMessage(msg); err != nil {
    103                                 uc.logger.Printf("failed to write message: %v", err)
     98                for {
     99                        var closed bool
     100                        select {
     101                        case msg := <-outgoing:
     102                                if uc.srv.Debug {
     103                                        uc.logger.Printf("sent: %v", msg)
     104                                }
     105                                if err := uc.irc.WriteMessage(msg); err != nil {
     106                                        uc.logger.Printf("failed to write message: %v", err)
     107                                }
     108                        case <-uc.closed:
     109                                closed = true
     110                        }
     111                        if closed {
     112                                break
    104113                        }
    105114                }
    … …  
    114123}
    115124
     125func (uc *upstreamConn) isClosed() bool {
     126        select {
     127        case <-uc.closed:
     128                return true
     129        default:
     130                return false
     131        }
     132}
     133
    116134func (uc *upstreamConn) Close() error {
    117         if uc.closed {
     135        if uc.isClosed() {
    118136                return fmt.Errorf("upstream connection already closed")
    119137        }
    120         close(uc.outgoing)
    121         uc.closed = true
     138        close(uc.closed)
    122139        return nil
    123140}

  • trunk/user.go

    r168 r175  
    6565                uc.register()
    6666
     67                // TODO: wait for the connection to be registered before adding it to
     68                // net, otherwise messages might be sent to it while still being
     69                // unauthenticated
    6770                net.lock.Lock()
    6871                net.conn = uc
    … …  
    113116        for _, network := range u.networks {
    114117                uc := network.upstream()
    115                 if uc == nil || !uc.registered || uc.closed {
     118                if uc == nil || !uc.registered {
    116119                        continue
    117120                }
    … …  
    153156                case eventUpstreamMessage:
    154157                        msg, uc := e.msg, e.uc
    155                         if uc.closed {
     158                        if uc.isClosed() {
    156159                                uc.logger.Printf("ignoring message on closed connection: %v", msg)
    157160                                break
Note: See TracChangeset for help on using the changeset viewer.