From ac16729f93f6d454f610b6499b4d7a87db380b3c Mon Sep 17 00:00:00 2001 From: Simon Ser Date: Wed, 5 Apr 2023 12:57:42 +0200 Subject: [PATCH] user: fix dup upstream connections due to race Closes: https://todo.sr.ht/~emersion/soju/207 --- user.go | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/user.go b/user.go index 02dd6f5..ac88ffe 100644 --- a/user.go +++ b/user.go @@ -227,6 +227,12 @@ func (net *network) runConn(ctx context.Context) error { } defer uc.Close() + // The context is cancelled by the caller when the network is stopped. + go func() { + <-ctx.Done() + uc.Close() + }() + if net.user.srv.Identd != nil { net.user.srv.Identd.Store(uc.RemoteAddr().String(), uc.LocalAddr().String(), userIdent(&net.user.User)) defer net.user.srv.Identd.Delete(uc.RemoteAddr().String(), uc.LocalAddr().String()) @@ -239,9 +245,6 @@ func (net *network) runConn(ctx context.Context) error { return fmt.Errorf("failed to register: %w", err) } - // TODO: this is racy with net.stopped. If the network is stopped - // before the user goroutine receives eventUpstreamConnected, the - // connection won't be closed. net.user.events <- eventUpstreamConnected{uc} defer func() { net.user.events <- eventUpstreamDisconnected{uc} @@ -259,6 +262,12 @@ func (net *network) run() { return } + ctx, cancel := context.WithCancel(context.TODO()) + go func() { + <-net.stopped + cancel() + }() + var lastTry time.Time backoff := newBackoffer(retryConnectMinDelay, retryConnectMaxDelay, retryConnectJitter) for { @@ -273,7 +282,7 @@ func (net *network) run() { } lastTry = time.Now() - if err := net.runConn(context.TODO()); err != nil { + if err := net.runConn(ctx); err != nil { text := err.Error() temp := true var regErr registrationError @@ -299,10 +308,6 @@ func (net *network) stop() { if !net.isStopped() { close(net.stopped) } - - if net.conn != nil { - net.conn.Close() - } } func (net *network) detach(ch *database.Channel) {