Fix time.After in inmemory/locker Scan Sleep Loop [mem]

[Effi-S]

Fixes #2127

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

Old issue reference

Sub-Task 6 — Fix time.After in inmemory/locker Scan Sleep Loop

Status: [ ] pending

#	Tag	Severity	Category	File	Key Lines	Issue
6	[RUN]	MEDIUM	Timer leak	selector/simple/inmemory/locker.go	268	time.After in scan sleep loop; should be Ticker

Intent

The scan goroutine in the in-memory locker uses time.After(d.sleepTimeout)
inside a for { select { } } loop. A new timer is allocated on every
iteration. Although the ctx.Done() branch provides an exit path, each
iteration before exit leaks the timer for up to sleepTimeout duration.

Expected Outcomes

• A single time.Ticker drives the scan sleep instead of per-iteration timers.
• ticker.Stop() is called via defer when the goroutine exits.

Todo List

1. Create ticker := time.NewTicker(d.sleepTimeout) before the outer loop.
2. Add defer ticker.Stop() immediately after.
3. Replace case <-time.After(d.sleepTimeout): with case <-ticker.C:.

Relevant Context

• File: token/services/selector/simple/inmemory/locker.go
• Lines 264–284: inner sleep loop in the scan goroutine.

---

Status in current code

Not a leak on Go 1.26, but the Ticker suggestion has independent merit. Line numbers have shifted: the loop is now at token/services/selector/simple/inmemory/locker.go:430, not 264–284.

The call is now at token/services/selector/simple/inmemory/locker.go:430
(case <-time.After(d.sleepTimeout):), inside the scan goroutine's sleep loop.

On the leak claim:

go.mod declares go 1.26.5. Per the Go 1.23 release notes: "Timers and Tickers that are no longer referred to by the program become eligible for garbage collection immediately, even if their Stop methods have not been called. Earlier versions of Go did not collect unstopped Timers until after they had fired and never collected unstopped Tickers." The timer returned by time.After becomes unreachable the moment the select completes, so it is collectable immediately rather than surviving until it fires. The document's premise — that the timer is retained for its full duration — describes pre-1.23 runtime behaviour and does not hold on this toolchain.

So "each iteration before exit leaks the timer for up to sleepTimeout duration" is not true
here — the timer is collectable as soon as its iteration's select returns.

This is the one item in the timer group where the suggested change is still worth making on
its own merits. A single time.NewTicker created before the loop with defer ticker.Stop()
is genuinely clearer than re-arming a timer every pass through a hot scan loop, and it
expresses the fixed-interval intent directly. It also avoids one allocation per scan cycle,
which is not a leak but is not free either.

Recommendation: fold into unrelated work in this file rather than tracking as a standalone
change. Kept open at Low for that purpose.

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

[AkramBitar]

See my comments

[AkramBitar]

@Effi-S,

Two items:

1. Ticker drops the pause between scans (blocking). A Ticker fires on a fixed grid with a 1-slot buffer. Once a scan exceeds sleepTimeout (one GetStatus per locked token, sleepTimeout is 2s in prod), the select finds a stale tick already waiting and rescans with zero pause — hammering the status provider. time.After always gave a full gap after the scan. Use a reusable Timer instead: same one allocation, original semantics.

timer := time.NewTimer(d.sleepTimeout)
defer timer.Stop()
// in the inner loop, before the select:
timer.Reset(d.sleepTimeout)
select {
case <-timer.C:
case <-ctx.Done():
    return
}

2. Panic on non-positive interval (minor). NewTicker/NewTimer panic when sleepTimeout <= 0; time.After did not. Clamp it in NewLocker.