-
Notifications
You must be signed in to change notification settings - Fork 0
Document goroutine leak contract in CallWithTimeout/RecvWithTimeout #8
Copy link
Copy link
Open
Labels
area/transportBinding, protocol, connector, or transport runtime changes.Binding, protocol, connector, or transport runtime changes.good first issueSmall, well-scoped tasks for new contributors.Small, well-scoped tasks for new contributors.help wantedLooking for community contributions.Looking for community contributions.kind/docsDocumentation, examples, or community-health updates.Documentation, examples, or community-health updates.priority/mediumNormal priority item.Normal priority item.
Metadata
Metadata
Assignees
Labels
area/transportBinding, protocol, connector, or transport runtime changes.Binding, protocol, connector, or transport runtime changes.good first issueSmall, well-scoped tasks for new contributors.Small, well-scoped tasks for new contributors.help wantedLooking for community contributions.Looking for community contributions.kind/docsDocumentation, examples, or community-health updates.Documentation, examples, or community-health updates.priority/mediumNormal priority item.Normal priority item.
Problem statement
CallWithTimeoutandRecvWithTimeoutinruntime/transport/connector/dial.gospawn a goroutine to run the callback and return early if the context deadline fires first. This is intentional and tested — but undocumented. If a callback ignores context cancellation, the goroutine is orphaned until the callback eventually returns (or never).Downstream consumers (operator, hub, SDK) need to know this contract to avoid goroutine accumulation during timeout storms.
Proposed change
CallWithTimeoutandRecvWithTimeoutexplaining the leak contract:(Optional) Consider adding a metric hook or callback for observability when the
<-callCtx.Done()branch fires before<-errCh, so downstream services can track orphaned goroutine count.Also consider documenting the
deriveCallContextnil-context guard behavior (line 88-90): it silently convertsniltocontext.Background(), which hides caller bugs. The Go stdlib panics on nil context. Document this or change it to panic.Affected area
runtime/*Compatibility / migration
Doc-only change. No behavior change unless the nil-context guard is converted to a panic (which would be a minor breaking change for callers passing nil).
Alternatives considered
Additional context
The tests
TestCallWithTimeoutReturnsDeadlineErrorWhenCallbackIgnoresContextandTestRecvWithTimeoutReturnsDeadlineErrorWhenCallbackIgnoresContextexplicitly exercise this behavior. Identified during code review.