From 9e187d7996dbdccdaf20d6dd80889e60c9dabc4f Mon Sep 17 00:00:00 2001 From: David Bimmler Date: Mon, 28 Oct 2024 11:34:50 +0100 Subject: [PATCH] shared_client: fix fail-safe mechanism If a shared client exchange fell into the fail-safe timeout of one minute, but the handler loop (due to either an error, closing or a _very_ delayed response) would write to the now reader-less channel, it would block all future progress of this shared client. Prevent that from happening by buffering the channel for the one message it will receive. Fixes: 4e6b438d (shared client: add fail-safe mechanism for stuck requests.) Signed-off-by: David Bimmler --- shared_client.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/shared_client.go b/shared_client.go index 621402fcf4..a4375f7082 100644 --- a/shared_client.go +++ b/shared_client.go @@ -304,7 +304,9 @@ func (c *SharedClient) ExchangeSharedContext(ctx context.Context, m *Msg) (r *Ms timeout := c.getTimeoutForRequest(c.Client.writeTimeout()) ctx, cancel := context.WithTimeout(ctx, timeout) defer cancel() - respCh := make(chan sharedClientResponse) + // The handler loop writes our response or an error to this channel. If we fall into the timeout + // below, the handling loop would block indefinitely on writing if this channel is not buffered. + respCh := make(chan sharedClientResponse, 1) select { case c.requests <- request{ctx: ctx, msg: m, ch: respCh}: case <-ctx.Done(): @@ -315,7 +317,8 @@ func (c *SharedClient) ExchangeSharedContext(ctx context.Context, m *Msg) (r *Ms select { case resp := <-respCh: return resp.msg, resp.rtt, resp.err - // This is just fail-safe mechanism in case there is another similar issue + // This is a fail-safe mechanism which prevents leaking this goroutine if the handling loop + // fails to write a response on our channel. case <-time.After(time.Minute): return nil, 0, fmt.Errorf("timeout waiting for response") }