Skip to content

Stop leaking goroutines when multiple submissions fail together - #272

Draft
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/submitter-errorchan-goroutine-leak
Draft

Stop leaking goroutines when multiple submissions fail together#272
damilolaedwards wants to merge 1 commit into
ethpandaops:masterfrom
damilolaedwards:fix/submitter-errorchan-goroutine-leak

Conversation

@damilolaedwards

Copy link
Copy Markdown
Contributor

Summary

  • The manager for a wallet's batch submission drains only the first error from a size-1 error channel, then stops listening. Every other failing sub-goroutine was doing an unconditional blocking send on that same channel, so once the manager stopped draining, they blocked forever with no way to release their semaphore slot or run their own cleanup.
  • Changed the send to a non-blocking best-effort select, so a sub-goroutine that fails after the manager has already moved on drops its error instead of hanging.

Test plan

  • Added a test that mirrors the manager/sub-goroutine channel structure with several sub-goroutines failing at once, and checks no goroutines are left running after the manager returns.
  • Verified the test fails against the old blocking send and passes with the fix, under -race.

Each per-wallet batch has a size-1 error channel that the manager goroutine
drains at most once before cancelling the batch and returning. A
sub-goroutine that hit a hard submission failure sent its error on that
channel unconditionally, which only works for the first one. Every other
sub-goroutine that failed around the same time blocked forever on that send,
since nothing drains the channel again once the manager has returned.

That leak was worse than an idle goroutine: the blocked sub-goroutine never
reached its own cleanup, so it never released its semaphore slot either.
Enough of these over a long run against a target that is failing under
load, which is exactly the condition this tool is built to create, would
eventually exhaust the submission semaphore and stall all further sends
from that wallet.

The send is now best effort. The error is still recorded in the per-wallet
results regardless of whether this particular send lands, so nothing is
lost other than a signal that was already redundant once one sub-goroutine
got through.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant