AnimatedVisualPlayer destructor bug #4955
Merged
Conversation
This file contains 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
ghost
added
the
StephenLPeters
added
area-AnimatedVisualPlayer
team-Controls
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
jevansaks
changed the title
jevansaks
reviewed
May 5, 2021
jevansaks
reviewed
May 5, 2021
|
|
jevansaks
approved these changes
May 14, 2021
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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.
This change should fix the internal bug
The problem is that calling
IsPlaying(false)inside theAnimatedVisualPlayers destructor causes a crash. Basically, our goal is to reduce a number of things that destructor is doing, ideally nothing. But the problem is that we want to destroyAnimationPlay m_nowPlayingobject (if it is alive) as well, butAnimatedVisualPlayer::PlayAsynckeeps this object alive, because it holds reference and awaits (co_await *thisPlay;) for it to callCompleteAwaits(). Also we need to releasem_batchlistener as well, because it holds weak reference to us (line ~130).So basically, we want to call
AnimationPlay ::Complete()but without touchingm_owner. But sinceAnimationPlaycallsm_owneronly if it is bound to it, we can detach it first, to ensure that we will not call anything in m_owner inside the destructor.