Skip to content

pool: receiveCalls swallows context cancellation — every generated method returns (nil, nil) when ctx is cancelled mid-call #28

Description

@iurii-ssv

Summary

pool/call.go's receiveCalls swallows context cancellation:

for {
    select {
    case <-ctx.Done():
        logging.FromContext(ctx).Debug(...)
        return nil            // <-- cancellation reported as success
    case log, ok := <-logs:
        if !ok {
            return nil
        }
        ...

When the caller's context is cancelled (or times out) while a call is in flight, call.Do returns a nil error even though no client ever succeeded. Every generated wrapper in pool/methods.go then returns its never-assigned zero result:

_result.err = m.callFunc(ctx, func(ctx context.Context, client beacon.Client) error { ... })
return _result.pp1, _result.err   // (nil, nil) when receiveCalls swallowed ctx.Done

So every pooled method can return (nil, nil), breaking the universal Go convention that a nil error implies a usable response. Any consumer that derefs resp.Data behind a plain err != nil check panics.

Production impact

This crashed ethereum2-monitor on hoodi: a redeploy's SIGTERM cancelled the root context mid-SignedBeaconBlock, the pool returned (nil, nil), and the block fetch nil-dereferenced (daemon panicked). Full analysis: ssvlabs/ethereum2-monitor#527. The monitor now carries defensive guards at every pooled call site (ssvlabs/ethereum2-monitor#528), but the contract violation originates here and affects every consumer of the pool.

Note the race shape: on cancellation, ctx.Done() becomes ready before the in-flight client calls unwind and post their CallLogs, so receiveCalls reliably takes the swallowing branch rather than aggregating the clients' context errors into a pool.Error.

Fix

On ctx.Done(), return ctx.Err() — or a *pool.Error carrying the accumulated trace with the context error included — instead of nil, restoring the invariant nil error ⇒ non-nil result. The case log, ok := <-logs: if !ok { return nil } branch deserves the same treatment for symmetry (unreachable today since close(calls) runs after receiveCalls returns, but the same hazard if that ordering ever changes).

Callers that treat cancellation as retryable (the monitor's fetch loops do errors.Is(err, context.Canceled)) work correctly with either shape, as long as the context error is reachable via errors.Is through whatever wrapper is chosen.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions