Skip to content

Fix Channel Leak in Multisig and BoolPolicy Spend Views [mem] - #2261

Draft
Effi-S wants to merge 1 commit into
mainfrom
fix-2131
Draft

Fix Channel Leak in Multisig and BoolPolicy Spend Views [mem]#2261
Effi-S wants to merge 1 commit into
mainfrom
fix-2131

Conversation

@Effi-S

@Effi-S Effi-S commented Aug 18, 2026

Copy link
Copy Markdown
Contributor

Fixes #2131

Document severity: HIGH · Verified priority: Low · Effort: Low · Type: Task

Old issue reference

Sub-Task 10 — Fix Channel Leak in Multisig and BoolPolicy Spend Views

Status: [ ] pending

# Tag Severity Category File Key Lines Issue
10 [RUN] HIGH Channel/goroutine leak ttx/multisig/spend.go + boolpolicy/spend.go 127–165 Answer goroutines blocked on send after ctx cancel

Intent

RequestSpendView.Call in both the multisig and boolpolicy packages fans
out to N goroutines, each sending its result to a shared answerChannel. When
the outer loop exits early (e.g., context cancellation or first error), the
remaining goroutines block forever on a send to answerChannel because no
receiver will ever drain the channel and the channel is never closed.

This pattern creates one goroutine leak per unread party response.

Expected Outcomes

  • All goroutines spawned for collecting answers always have a path to exit.
  • answerChannel is never left with blocked senders after Call returns.

Todo List

  1. Make answerChannel buffered to the exact number of goroutines spawned
    (counter), so that senders can complete their send even if the receiver
    has exited. (The current code already sizes the channel to len(c.parties)
    but counter may be smaller when some parties are "me".)
  2. After the receive loop exits (whether by context cancel or error), drain
    any remaining items from answerChannel in a non-blocking loop so the
    goroutines that have already sent can be GC'd.
  3. Apply the fix to both multisig/spend.go and boolpolicy/spend.go.

Relevant Context

  • File: token/services/ttx/multisig/spend.go
  • Lines 127–165: answerChannel creation and receive loop.
  • File: token/services/ttx/boolpolicy/spend.go
  • Lines 143–169: identical pattern.

Status in current code

Already fixed — filed for double-checking only. The answerChannel pattern the document describes no longer exists in either file.

Both call sites were refactored to a shared helper. The bare answerChannel fan-out
is gone:

  • token/services/ttx/multisig/spend.go:130
  • token/services/ttx/boolpolicy/spend.go:154

both now construct

collector := utils.NewAnswersCollector[string, *SpendResponse](len(c.parties), c.timeout)

and collect via collector.Collect(context.Context(), counter) (multisig/spend.go:152,
boolpolicy/spend.go:163). The helper is token/services/utils/collector.go.

Why this closes both remediation items:

Todo item 1 (buffer to the spawned count). collector.go:59 allocates
make(chan Answer[K, T], capacity) — buffered to the full capacity passed in. Send
(:66-68) therefore never blocks as long as senders do not exceed capacity, which the callers
guarantee by passing len(c.parties) while spawning at most counter <= len(c.parties)
goroutines. The document's specific worry — counter being smaller than len(c.parties) when
some parties are "me" — is safe in this direction: the buffer is sized to the larger number,
so every spawned sender has a slot.

Todo item 2 (no blocked senders after Call returns). Because the channel is buffered to
capacity, an in-flight worker completes its Send into the buffer and exits even when
Collect has already returned on timeout or cancellation. No explicit drain is needed — the
un-received answers and the channel itself simply become garbage once the collector goes out
of scope. The type's doc comment at collector.go:37-45 states this is the intent: "The
underlying channel is buffered to the collector's capacity, so workers that are still in
flight when Collect returns (on timeout or cancellation) can always complete their Send and
exit without leaking a goroutine."

Bonus — the collector cannot itself hang. Collect (:73-91) selects on three cases:
the answer channel, ctx.Done(), and timer.C from a time.NewTimer(c.timeout) with
defer timer.Stop() at :76-77. NewAnswersCollector also substitutes
DefaultAnswersCollectorTimeout (30 s, :20) when a non-positive timeout is passed, so a
collector is never accidentally unbounded. The original code's failure mode — a bare receive
that waits forever — is structurally impossible.

Recommended action: verify the above and close as completed. No code change required.

Severity and priority

The source document rated this HIGH. Re-checked against this repository at
origin/main with go 1.26.5 on 2026-08-04, the priority is Low for the
reasons in Status in current code above.

Effort

Low

@Effi-S Effi-S added this to the Q3/26 milestone Aug 18, 2026
@Effi-S Effi-S self-assigned this Aug 18, 2026
Signed-off-by: Effi-S <effi.szt@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix Channel Leak in Multisig and BoolPolicy Spend Views [mem]

2 participants