Fix Fire-and-Forget Goroutine in utils/view.RunView [mem] #2134

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

Old issue reference

Sub-Task 13 — Fix Fire-and-Forget Goroutine in utils/view.RunView

Status: [ ] pending

Intent

RunView in token/services/utils/view/view.go spawns a goroutine with no
wait mechanism, no stop channel, and no way for the caller to know when it
completes. If the goroutine holds open DB transactions, network sessions, or
traces, those resources are leaked until the goroutine eventually finishes
(or never does if the underlying view blocks).

Expected Outcomes

• Callers can choose to wait for the view to finish (even if they ignore
  the result).
• The goroutine's panic recovery captures any open spans or contexts so
  they are properly closed.

Todo List

1. Return a <-chan error from RunView so callers can optionally wait.
2. Close the channel (with nil or the error) when the goroutine finishes.
3. Alternatively, accept a context.Context parameter and propagate
   cancellation into the view execution.
4. Confirm that all call sites of RunView can handle the returned channel
   (they may simply ignore it if fire-and-forget is intentional).

Relevant Context

• File: token/services/utils/view/view.go
• Lines 46–63: RunView function.

Status in current code

By-design fire-and-forget — the code is as described, but the behaviour is the function's documented contract rather than a defect. One incidental dead-code nit noted below.

Confirmed unchanged at token/services/utils/view/view.go:46-63. The goroutine is
real and nothing waits on it.

Why this is not a leak to fix here. Fire-and-forget is the stated contract, not an
oversight. The doc comment at :45 reads "RunView runs passed view within the passed Context
and using the passed options in a separate goroutine" — running detached is the entire
purpose of the function, and callers that want a result already have context.RunView or the
RunViewWithTimeout wrapper at :30. The goroutine is also not unsupervised in the way the
document implies: the inner defer recover() at :53-57 catches panics from the view and
logs them, and :59-61 logs any returned error. So it terminates and reports on both the
success and failure paths.

The failure mode the document describes — held-open DB transactions, sessions or spans —
requires a view that blocks forever. That is a defect in that view, and the fix belongs
there; making RunView return a channel does not stop a blocked view from blocking, it only
gives a caller the option to block alongside it. Todo item 4 concedes the point ("they may
simply ignore it if fire-and-forget is intentional").

Changing the signature would also touch every call site to satisfy linters about the discarded
channel, for no behavioural gain in the fire-and-forget case that is the norm.

One genuine nit if this file is edited for another reason. The outer deferred recover()
at :47-51 is dead code. It guards only the go statement, which cannot panic; the closure
runs on a different goroutine, so a panic inside the view can never unwind through this
defer. The inner recovery at :53-57 is the one actually doing the work. The outer block is
harmless but misleading to a reader who assumes it provides real protection.

Recommended action: close as working-as-intended, or keep at Low solely to carry the
dead-defer cleanup.

Severity and priority

The source document rated this MEDIUM. 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