Repository navigation
Fix BSAnimationGraphManager Refcount Leaks in Save/Load Animation Variables and DrawAnimDebugView - #920
Merged
miredirex merged 4 commits intoOct 14, 2026
Conversation
…ptr and deleter which releases the reference. This was not previously done on early returns in TESObjectREFR save/load variables, and never in DrawAnimDebugView.
… dtor. This passes flag 1 to the dtor and frees the memory via game delete operator, while before flag was 0 and memory not freed.
Contributor
Author
|
IAnimationGraphManagerHolder::GetBSAnimationGraph incrementing refcount is when smart pointer is created. |
miredirex
requested changes
Oct 9, 2026
miredirex
left a comment
miredirex
left a comment
Collaborator
There was a problem hiding this comment.
Tested this, and it crashed the second client right after connecting, possibly a use-after-free https://pastebin.com/nALKQfYz
Contributor
Author
|
Fixed crash, will figure out how to update |
Contributor
Author
|
ok fix commit here automatically, should be ready? |
Port trimmed versions of CommonLibSSE's BSTSmartPointer and BSIntrusiveRefCounted into BSCore
absol89
added a commit
to absol89/TiltedEvolutionScriptFixes
that referenced
this pull request
Oct 17, 2026
…les" This reverts commit 13afaff. The bandaid removed the manual pManager->Release() left behind by the earlier tiltedphoques#920 version (3c7df15), which double-released the graph manager. Upstream tiltedphoques#920 (next commit) removes that call itself as part of moving to BSTSmartPointer, so the bandaid is no longer needed as a separate change. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Mdy6FmrJTc1m7KuVM1n3H
absol89
pushed a commit
to absol89/TiltedEvolutionScriptFixes
that referenced
this pull request
Oct 17, 2026
…rs (tiltedphoques#920) * Guarantee BSAnimationGraphManager ref-count Release with std::unique_ptr and deleter which releases the reference. This was not previously done on early returns in TESObjectREFR save/load variables, and never in DrawAnimDebugView. * Fix BSAnimationGraphManager::Release to use delete instead of calling dtor. This passes flag 1 to the dtor and frees the memory via game delete operator, while before flag was 0 and memory not freed. * Fix: forgot to get rid of manual Release call in LoadAnimationVariables * Replace unique_ptr graph manager handle with BSTSmartPointer Port trimmed versions of CommonLibSSE's BSTSmartPointer and BSIntrusiveRefCounted into BSCore --------- Co-authored-by: Daniil Zakharov <miredirex@gmail.com> (cherry picked from commit 390071c on tiltedphoques/TiltedEvolution dev) Upgrades the earlier version of tiltedphoques#920 on this branch (8364950, 3c7df15) to the merged upstream solution: BSAnimationGraphManager now derives from BSIntrusiveRefCounted and GetAnimationGraphManagerPtr returns a BSTSmartPointer. IAnimationGraphManagerHolder.h/.cpp taken from upstream (the auto-merge had left both the unique_ptr and BSTSmartPointer overloads). Save/LoadAnimationVariables match upstream exactly. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019Mdy6FmrJTc1m7KuVM1n3H
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
2 participants
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.
IAnimationGraphManagerHolder::GetBSAnimationGraph increments the animation graph refcount, and the out parameter is actually a BSTSmartPointer in-game.
Previously, early returns when saving/loading animation variables (chiefly when no descriptor found) would not release the animation graph manager. DrawAnimDebugView never released it. Result is that refcount grows massively and can never hit 0 (unless game is left running long enough for integer overflow I guess). This fixes it without a full BSIntrusiveRefCounted + BSTSmartPointer refactor/implementation.
Latent issue was the actual Release() implementation, which called the destructor directly (passing flag 0), instead of using delete (passing flag 1) like the game. Although I'm unsure if that actually happened in practice, it would result in the game never freeing the manager's memory. Either way,
delete thismatches the game's logic.This is an immediate fix, since I'm unsure the general BSTSmartPointer + BSIntrusiveRefCount implementation is considered worth doing. If it is, that is an alternative, or we can have this until then.