Repository navigation
TextureCache: fix use-after-free in CleanTimer() - #27972
Merged
Merged
Conversation
CleanTimer() was submitting a [this] lambda to the global job manager (CServiceBroker::GetJobManager()). Deinitialize() only called CancelJobs() which clears the local CJobQueue — it has no effect on jobs already handed to the global manager. Additionally, m_cleanTimer was never stopped in Deinitialize(), so the timer could fire and enqueue a new global job after teardown had begun. Fix by: - Stopping m_cleanTimer in Deinitialize() before CancelJobs(), preventing any further CleanTimer() invocations after teardown starts. - Submitting through the local CJobQueue (Submit()) instead of the global job manager, so CancelJobs() and the CJobQueue destructor properly govern the job lifetime and the [this] lambda cannot outlive *this. Confirmed with AddressSanitizer: heap-use-after-free on m_cleaningInProgress when the global job worker thread runs the lambda after CTextureCache is freed. Co-Authored-By: Claude Sonnet 4.6 <[email protected]>
Contributor
Review Details
|
![kodiai[bot]](https://avatars.githubusercontent.com/in/2832814?s=60&v=4){kind=small}
neo1973
approved these changes
Mar 10, 2026
neo1973
left a comment
neo1973
left a comment
Member
There was a problem hiding this comment.
Thank you, I think this makes sense.
3 of 7 tasks
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.
Closes #27971
What
CTextureCache::CleanTimer()was submitting a[this]lambda to the global job manager (CServiceBroker::GetJobManager()).Deinitialize()only calledCancelJobs(), which clears the localCJobQueue— it has no effect on jobs already handed to the global manager. Additionally,m_cleanTimerwas never stopped inDeinitialize(), so the timer could fire and enqueue a new global job after teardown had begun.After
CTextureCacheis destroyed, the pending lambda runs on the global job manager's worker thread and accesses freedm_cleaningInProgressandm_cleanTimermembers (heap-use-after-free).Fix
m_cleanTimer.Stop(true)at the top ofDeinitialize()so no furtherCleanTimer()invocations can be enqueued after teardown starts.CleanTimer()to submit viaSubmit()(the localCJobQueue) instead ofCServiceBroker::GetJobManager(), soCancelJobs()and theCJobQueuedestructor properly govern the lambda's lifetime — the[this]capture cannot outlive*this.Testing
Confirmed with AddressSanitizer: heap-use-after-free on
m_cleaningInProgressreproduced reliably before the fix, and is absent after.