Repository navigation
fix: free the DataPacks of retry timers killed by a map change - #102
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
A failed query hands its
DataPacktoCreateTimer(..., 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 leakedDataPackper retry pending at map end.TIMER_DATA_HNDL_CLOSE/CreateDataTimercannot 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 10CreateTimer(g_cvar_SQLRetryTime...)calls that carry a pack. It records the pack ing_alRetryPacksbefore creating the timer.SQL_BeginRequest(timer, pack, counters, client)(the formerResetRetryBudget) is still the first call of the 10 request entry points. Whentimeris 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.OnMapEndcallsSQL_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.g_smReplacePendingTriggers). Its retry dying with the map would have leaked it too, soOnMapEndalso callsClearReplaceReservations(). An insert still in flight is not affected: its callback only decrements for a reservation it finds.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.
sm logs, and the handle count (sm_dump_handlesbefore and after) does not grow over repeated cycles.🤖 Generated with Claude Code