From 5f3a53f01bddba6584f26cd9d02340f1c3f8ff91 Mon Sep 17 00:00:00 2001 From: Spike Curtis Date: Tue, 25 Mar 2025 16:45:45 +0400 Subject: [PATCH] fix: fix race condition in pubsub startup (#17088) fixes https://github.com/coder/internal/issues/525 If the context is canceled, the goroutine that is supposed to read from the `errCh` could exit prematurely, leading to a goroutine leak. Refactors this code so it cannot block. --- coderd/database/pubsub/pubsub.go | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/coderd/database/pubsub/pubsub.go b/coderd/database/pubsub/pubsub.go index 6823dc0188..8019754e15 100644 --- a/coderd/database/pubsub/pubsub.go +++ b/coderd/database/pubsub/pubsub.go @@ -492,7 +492,6 @@ func (p *PGPubsub) startListener(ctx context.Context, connectURL string) error { p.connected.Set(0) // Creates a new listener using pq. var ( - errCh = make(chan error) dialer = logDialer{ logger: p.logger, // pq.defaultDialer uses a zero net.Dialer as well. @@ -525,6 +524,10 @@ func (p *PGPubsub) startListener(ctx context.Context, connectURL string) error { dc.Dialer(dialer) } + var ( + errCh = make(chan error, 1) + sentErrCh = false + ) p.pgListener = pqListenerShim{ Listener: pq.NewConnectorListener(connector, connectURL, time.Second, time.Minute, func(t pq.ListenerEventType, err error) { switch t { @@ -541,18 +544,16 @@ func (p *PGPubsub) startListener(ctx context.Context, connectURL string) error { p.logger.Error(ctx, "pubsub failed to connect to postgres", slog.Error(err)) } // This callback gets events whenever the connection state changes. - // Don't send if the errChannel has already been closed. - select { - case <-errCh: + // Only send the first error. + if sentErrCh { return - default: - errCh <- err - close(errCh) } + errCh <- err // won't block because we are buffered. + sentErrCh = true }), } select { - case err := <-errCh: + case err = <-errCh: if err != nil { _ = p.pgListener.Close() return xerrors.Errorf("create pq listener: %w", err)