Fix memory leak when prompt isn't explicitly dismissed#113
Merged
sjwall merged 2 commits intosjwall:masterfrom May 5, 2018
SUPERCILEX:leak
Merged
Fix memory leak when prompt isn't explicitly dismissed#113sjwall merged 2 commits intosjwall:masterfrom SUPERCILEX:leak
sjwall merged 2 commits intosjwall:masterfrom
SUPERCILEX:leak
Conversation
Signed-off-by: Alex Saveau <saveau.alexandre@gmail.com>
Codecov Report
@@ Coverage Diff @@
## master #113 +/- ##
============================================
- Coverage 95.88% 95.62% -0.27%
Complexity 334 334
============================================
Files 14 14
Lines 1070 1074 +4
Branches 125 125
============================================
+ Hits 1026 1027 +1
- Misses 21 24 +3
Partials 23 23
Continue to review full report at Codecov.
|
sjwall
requested changes
May 4, 2018
| protected void onDetachedFromWindow() | ||
| { | ||
| super.onDetachedFromWindow(); | ||
| mPrompt.cleanUpAnimation(); |
Owner
There was a problem hiding this comment.
Is there a reason that this isn't a call to cleanUpPrompt?
Contributor
Author
There was a problem hiding this comment.
cleanUpPrompt also notifies state changes which seems like we'd be introducing a race condition... I just realized I didn't share the leak with you, it's just the animator stuff which is leaking:
In com.supercilex.robotscouter.debug:2.3.1-DEBUG:1.
* com.supercilex.robotscouter.HomeActivity has leaked:
* GC ROOT android.widget.Toast$TN.mNextView
* references android.widget.LinearLayout.mParent
* references android.view.ViewRootImpl.mChoreographer
* references android.view.Choreographer.mCallbackQueues
* references array android.view.Choreographer$CallbackQueue[].[1]
* references android.view.Choreographer$CallbackQueue.mHead
* references android.view.Choreographer$CallbackRecord.action
* references android.animation.AnimationHandler$1.this$0 (anonymous implementation of android.view.Choreographer$FrameCallback)
* references android.animation.AnimationHandler.mAnimationCallbacks
* references java.util.ArrayList.elementData
* references array java.lang.Object[].[0]
* references android.animation.ValueAnimator.mUpdateListeners
* references java.util.ArrayList.elementData
* references array java.lang.Object[].[0]
* references uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt$10.this$0 (anonymous implementation of android.animation.ValueAnimator$AnimatorUpdateListener)
* references uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt.mView
* references uk.co.samuelwall.materialtaptargetprompt.MaterialTapTargetPrompt$PromptView.mContext
* leaks com.supercilex.robotscouter.HomeActivity instance
* Retaining: 6.5 kB.
* Reference Key: 8200773e-a087-4f54-8dd7-9fe208a43681
* Device: Google google Pixel 2 XL taimen
* Android Version: 8.1.0 API: 27 LeakCanary: 1.5.4 74837f0
* Durations: watch=5315ms, gc=172ms, heap dump=1438ms, analysis=154338ms
| { | ||
| final ResourceFinder resourceFinder = promptOptions.getResourceFinder(); | ||
| mView = new PromptView(resourceFinder.getContext()); | ||
| mView.mPrompt = this; |
Owner
There was a problem hiding this comment.
Should this reference be set to null in cleanUpPrompt?
Contributor
Author
There was a problem hiding this comment.
We can, but it isn't necessary. The issue is the prompt holding on to the view, not the other way around.
Owner
|
Thanks again for your work and supplying the memory leak info |
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.
The animation listener doesn't die with the view apparently (which leaks it).