Zombie JumpingBeans instance causes java.lang.StackOverflowError
Closed this issue · 7 comments
Hi guys,
I'm using your library for an app project and really like it so far.
However, I just stumbled over a rather tricky issue: I'm triggering a JumpingBeans animation on a single TextView from several places in my fragment, like so:
loadingDots = new JumpingBeans.Builder().appendJumpingDots(tvLoading).build();When the fragment is paused, I cancel any possibly active animation by calling:
if (loadingDots != null)
loadingDots.stopJumping();Now, by accident, I actually created a JumpingBeans instance twice, i.e. I assigned a new JumpingBeans instance to loadingDots without calling .stopJumping() on the old one. This doesn't cause any problem until the users leaves the current activity and transitions to a new one. That's when the zombie JumpingBeans instance (which apparently doesn't get GC'ed) actually takes down the entire app with a java.lang.StackOverflowError. The culprit is JumpingBeansSpan, which makes an infinitely recursive call to animation.start() when the TextView still exists but the span is no longer attached to it:
@Override
public void onAnimationUpdate(ValueAnimator animation) {
// No need for synchronization as this always run on main thread anyway
TextView v = textView.get();
if (v != null) {
if (isAttachedToHierarchy(v)) {
shift = (int) animation.getAnimatedValue();
v.invalidate();
} else {
// If there is a zombie JumpingBeans instance that is not attached
// to a view, this code block will cause infinite recursion and a
// java.lang.StackOverflowError.
animation.setCurrentPlayTime(0);
animation.start();
}
} else {
...
}
}I am aware that this error came from improper use of the library (now I actually stop and clean up any existent JumpingBeans instance before creating a new one and it works fine). Still, the consequences of this possibly common pitfall seem a bit harsh, so I'd like to suggest the following:
- Detect and prevent recursive starting of the animation in https://github.com/frakbot/JumpingBeans/blob/master/lib/src/main/java/net/frakbot/jumpingbeans/JumpingBeansSpan.java#L85-L86 - this would mitigate the
java.lang.StackOverflowErrorand possibly resulting app crashes. - Even better (if possible): Clear any old JumpingBeans instances that hold a WeakReference to a TextView when a new instance is attached to the same TextView.
Unfortunately, I don't have time to work out a PR at the moment, but I thought I'd share this issue anyway.
Hi @klaas-loclet, and thanks for the really detailed issue explaination. I'll take a look at it and issue a fix ASAP. I'm also taking the opportunity to update all dependencies and release a new library version, 1.1.0.
Hey @rock3r, great - thanks! Looking forward to the new release. Any chance you could deploy it on MavenCentral / JCenter, too? I was hoping to be able to import it via gradle like
compile 'net.frakbot.jumpingbeans:jumpingbeans:1.1.0'
Currently, it doesn't seem to be available there.
Hmm that's weird, it should be on mavenCentral(). I'll double check. I plan on releasing the new version on jcenter(), anyway.
Hmm yeah looks like it's not there. Oh well, 1.2.0 will be on jcenter() anyway.
@klaas-loclet can you try to see if you can still reproduce the issue you were having with the current HEAD of the master branch? I've cleaned up and refactored the code, it should be fixed now. Or at least, I can't reproduce issues by purposely not stopping the jumping beans.
Hey @rock3r, I just checked the latest update - that fixed it. No StackOverflowError to be seen, instead just a nice error message on the logcat. And I quite like the function name at https://github.com/frakbot/JumpingBeans/blob/master/lib/src/main/java/net/frakbot/jumpingbeans/JumpingBeansSpan.java#L85 😄
Thanks for the quick fix!