plattysoft / Leonids

A Particle System for standard Android UI: http://plattysoft.github.io/Leonids/
Apache License 2.0
2.28k stars 397 forks source link

Crash on mDrawingView.postInvalidate(); (We have tens of millions of users) #85

Open ynkm169 opened 7 years ago

ynkm169 commented 7 years ago

Stack trace: java.lang.NullPointerException: Attempt to invoke virtual method 'void android.view.View.postInvalidate()' on a null object reference com.plattysoft.leonids.ParticleSystem.onUpdate(ParticleSystem.java:683) com.plattysoft.leonids.ParticleSystem.access$000(ParticleSystem.java:38) access$100 com.plattysoft.leonids.ParticleSystem$ParticleTimerTask.run(ParticleSystem.java:82) java.util.TimerThread.mainLoop(TimerThread.java:555) java.util.TimerThread.run(TimerThread.java:505)

Hi,

We use this library to do some fancy particle effects for our app. We received crash report on this line. How could we possible fix this? An obvious fix is to add if(mDrawingView != null), however, I am not sure what consequence this might cause.

Thanks Yujiao

plattysoft commented 7 years ago

That's strange because that view is created by the ParticleSystem and since it holds a reference to it, the object should not be deallocated.

The most logical explanation is that the ParticleSystem is still trying to render something when the views are deallocated (but it is usually destroyed within the Activity lifecycle),

I think the safest option is to explicitly call cancel() inside onDestroy() of your Activity / Fragment to make sure that everything stops nicely.

Let me know if this helps.

plattysoft commented 7 years ago

OTOH, checking if the view is null before calling postInvalidate should be safe. If the view is null it is not going to draw anything. Also, in that case, it would make sense to call cancel. I may add that as a safety measure :)

ynkm169 commented 7 years ago

Hi plattysoft,

Thanks for helping! Could you please add null check? We use gradle so we can't change this locally.

We call cancel() in a few places. It seems this in turn calls cleanupAnimation() which sets mDrawingView null.

We also sometimes call cancel() in a different thread by postDelayed.

Yujiao

plattysoft commented 7 years ago

Maybe that's the problem. Let me explain it:

cancel is designed to abruptly end the animation and stop everything, getting ready for garbage collection. You are not supposed to reuse a ParticleSystem after you call cancel, you are supposed to create a new one.

stopEmitting, on the other hand, stops emitting new particles, but the ones that exist do continue to animate. It is safe to call startEmitting again after stopEmitting.

To be fair, the ParticleSystem should throw an exception if startEmitting is called after cancel, that could be an interesting improvement.

Please let me know if this is your case.

ynkm169 commented 7 years ago

Hi plattysoft,

I will debug our app a bit more next week to reproduce the exact circumstance that caused crash and let you know what happened.

Thanks

ynkm169 commented 7 years ago

Hi plattysoft,

We end up using stopEmitting() only, and removed cancel(). The original code was calling both stopEmitting and cancel to completely stop the particleSystem.

We will monitor crash report for similar issues once our new app is released.

Thanks Yujiao

plattysoft commented 7 years ago

Thanks for letting me know. I've been meaning to ask you about that for a couple of days.

I expect that crash to go away, or at least to decrease the prominence. Essentially if you reused a ParticleSystem after calling cancel, it would crash, so calling just stopEmitting should be safer.

Keep me updated either way.

ynkm169 commented 7 years ago

We use postDelay in UI thread. There is no synchronization issues.

renyuzhuo commented 6 years ago

Yes, I get the same NullPointerException:

ParticleSystem system = new ParticleSystem(……);
system.emit(view, 2); // We also postDelay in UI thread
    /** 
          -> emitWithGravity
          -> startEmiting
          -> mTimer.schedule(mTimerTask, 0, TIMMERTASK_INTERVAL);
          -> ParticleTimerTask:run()
          -> ps.onUpdate(ps.mCurrentTime);
          -> mDrawingView.postInvalidate();
          -> NullPointerException (mDrawingView is null)
    **/

though the view is NOT null, there is still has the crash. Thank you.