Skip to content

fix: prevent timer leak in GTFS retry loop#801

Open
ashnaaseth2325-oss wants to merge 1 commit intoOneBusAway:mainfrom
ashnaaseth2325-oss:fix/stop-timer-on-ctx-cancel
Open

fix: prevent timer leak in GTFS retry loop#801
ashnaaseth2325-oss wants to merge 1 commit intoOneBusAway:mainfrom
ashnaaseth2325-oss:fix/stop-timer-on-ctx-cancel

Conversation

@ashnaaseth2325-oss
Copy link
Copy Markdown
Contributor

SUMMARY

This PR fixes a timer lifecycle issue in InitGTFSManager where retry backoff used time.After, which cannot be stopped on context cancellation. It replaces those calls with time.NewTimer to ensure proper cleanup.

The changes affect internal/gtfs/gtfs_manager.go, specifically the retry sleep logic in the GTFS initialization flow.


FIX

// Before
select {
case <-ctx.Done():
    return nil, ctx.Err()
case <-time.After(delay):
}

// After
retryTimer := time.NewTimer(delay)
select {
case <-ctx.Done():
    retryTimer.Stop()
    return nil, ctx.Err()
case <-retryTimer.C:
}

VERIFICATION

Test by starting the service with a failing GTFS endpoint so it enters the retry loop, then cancel the context (e.g., send SIGTERM) while it is sleeping. The function should exit immediately with ctx.Err() instead of waiting for the delay. Retry behavior should remain unchanged when the context is not cancelled, with no lingering timers.

Replace time.After with time.NewTimer in the three backoff
sleeps inside InitGTFSManager so the timer is stopped
immediately when the context is cancelled, instead of
leaking in the runtime heap until it fires.

Signed-off-by: ashnaaseth2325-oss <ashnaaseth2325@gmail.com>
@ashnaaseth2325-oss
Copy link
Copy Markdown
Contributor Author

Hello @aaronbrethorst
Kindly review this PR whenever you have time.

@ashnaaseth2325-oss
Copy link
Copy Markdown
Contributor Author

Hi @aaronbrethorst
Just a gentle ping whenever you have time

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