Skip to content

fix: free the DataPacks of retry timers killed by a map change - #102

Merged
Rushaway merged 3 commits into
masterfrom
fix/retry-pack-leak
Oct 20, 2026
Merged

Rushaway merged 3 commits into
masterfrom
fix/retry-pack-leak

Conversation

@Rushaway

@Rushaway Rushaway commented Oct 20, 2026 •

Copy link
Copy Markdown
Member

Summary

A failed query hands its DataPack to CreateTimer(..., TIMER_FLAG_NO_MAPCHANGE) for the retry. A map change kills that timer, so the pack was never given back to a request and never deleted: one leaked DataPack per retry pending at map end. TIMER_DATA_HNDL_CLOSE / CreateDataTimer cannot fix it, they close the pack when the timer ends and these packs are reused after the timer fires (they become the data of the next query).

Changes

  • SQL_ScheduleRetry(callback, pack) replaces the 10 CreateTimer(g_cvar_SQLRetryTime...) calls that carry a pack. It records the pack in g_alRetryPacks before creating the timer.
  • SQL_BeginRequest(timer, pack, counters, client) (the former ResetRetryBudget) is still the first call of the 10 request entry points. When timer is a real handle the retry timer fired, so the pack goes back to the request and is removed from the registry; otherwise it is a new request and gets its fresh retry budget as before.
  • OnMapEnd calls SQL_ClearPendingRetries(), which deletes the packs whose timer is about to die with the map. A pack is in the registry only while its timer is pending, so nothing in flight is freed.
  • A pending replace insert also owns a reservation of the list capacity (g_smReplacePendingTriggers). Its retry dying with the map would have leaked it too, so OnMapEnd also calls ClearReplaceReservations(). An insert still in flight is not affected: its callback only decrements for a reservation it finds.
  • The replace list retry (SQLSelect_Replace) and the startup retries carry no pack and are unchanged.

Testing

Compiled with spcomp 1.12.0.7255 against the same dependency set as CI, no warnings. Not exercised on a live server, the runtime steps below are for the reviewer.

  • With the database down, trigger a query (for example change a tag color) and change the map before the retry fires: no error in sm logs, and the handle count (sm_dump_handles before and after) does not grow over repeated cycles.
  • A retry that fires normally (database back before the timer) still runs its query and the tag is saved.

🤖 Generated with Claude Code

Copilot AI balanced review requested due to automatic review settings October 20, 2026 18:47

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@Rushaway
Rushaway merged commit aed3e82 into master Oct 20, 2026
9 checks passed
@Rushaway
Rushaway deleted the fix/retry-pack-leak branch October 20, 2026 18:49
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.

2 participants