Crash if push another controller from onAttach()

[fedoretskn]

App crashes if push another controller form onAttach() without animation if current controller has RecyclerView in layout.

Steps to reproduce:

1. open controller with RecyclerView in layout.
2. push another controller from onAttach() method in the same navigation stack
3. crash the app

Log:

FATAL EXCEPTION: main
                                                 Process: com.test.conductor.conductortest, PID: 3562
                                                 java.lang.NullPointerException: Attempt to invoke virtual method 'void android.support.v7.widget.GapWorker.remove(android.support.v7.widget.RecyclerView)' on a null object reference
                                                     at android.support.v7.widget.RecyclerView.onDetachedFromWindow(RecyclerView.java:2479)
                                                     at android.view.View.dispatchDetachedFromWindow(View.java:14555)
                                                     at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3071)
                                                     at android.view.ViewGroup.dispatchDetachedFromWindow(ViewGroup.java:3063)
                                                     at android.view.ViewGroup.removeViewInternal(ViewGroup.java:4603)
                                                     at android.view.ViewGroup.removeViewInternal(ViewGroup.java:4576)
                                                     at android.view.ViewGroup.removeView(ViewGroup.java:4507)
                                                     at com.bluelinelabs.conductor.changehandler.SimpleSwapChangeHandler.performChange(SimpleSwapChangeHandler.java:69)
                                                     at com.bluelinelabs.conductor.ControllerChangeHandler.executeChange(ControllerChangeHandler.java:173)
                                                     at com.bluelinelabs.conductor.Router.performControllerChange(Router.java:652)
                                                     at com.bluelinelabs.conductor.Router.performControllerChange(Router.java:640)
                                                     at com.bluelinelabs.conductor.Router.pushController(Router.java:153)
                                                     at com.test.conductor.conductortest.controllers.ListController.onAttach(ListController.java:34)
                                                     at com.bluelinelabs.conductor.Controller.attach(Controller.java:791)
                                                     at com.bluelinelabs.conductor.Controller.access$600(Controller.java:42)
                                                     at com.bluelinelabs.conductor.Controller$7.onViewAttachedToWindow(Controller.java:903)
                                                     at android.view.View.dispatchAttachedToWindow(View.java:14525)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2836)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2843)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2843)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2843)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2843)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2843)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2843)
                                                     at android.view.ViewGroup.dispatchAttachedToWindow(ViewGroup.java:2843)
                                                     at android.view.ViewRootImpl.performTraversals(ViewRootImpl.java:1364)
                                                     at android.view.ViewRootImpl.doTraversal(ViewRootImpl.java:1107)
                                                     at android.view.ViewRootImpl$TraversalRunnable.run(ViewRootImpl.java:6013)
                                                     at android.view.Choreographer$CallbackRecord.run(Choreographer.java:858)
                                                     at android.view.Choreographer.doCallbacks(Choreographer.java:670)
                                                     at android.view.Choreographer.doFrame(Choreographer.java:606)
                                                     at android.view.Choreographer$FrameDisplayEventReceiver.run(Choreographer.java:844)
                                                     at android.os.Handler.handleCallback(Handler.java:739)
                                                     at android.os.Handler.dispatchMessage(Handler.java:95)
                                                     at android.os.Looper.loop(Looper.java:148)
                                                     at android.app.ActivityThread.main(ActivityThread.java:5417)
                                                     at java.lang.reflect.Method.invoke(Native Method)
                                                     at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:726)
                                                     at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:616)

I've created test project for reproduce this issue: https://github.com/fedoretskn/conductor_list_test

It happens because onAttach() is invoked from onAttachedToWindow of parent view before was called onAttachedToWindow of RecyclerView when should be initialized gapWorker. So when the controller was detached then android calls onDetachFromWindow method of RecyclerView which calls remove() method of gapWorker which is null.

[EricKuck]

This seems like it's a bug in RecyclerView, doesn't it? The same thing would happen if you added, then removed it from a view right away. Is this actually related to Conductor?

[fedoretskn]

I created test view for logging onAttach state changes. New commit is available now at my repository.

This is log of my test

D/TestAttachView: onViewDetachedFromWindow
D/TestAttachView: onViewAttachedToWindow

First invoked onViewDetachedFromWindow and after that onViewAttachedToWindow
It seems this is wrong behavior.
Controller's onAttach() should be called after as all internal views have been attached.

If you uncomment my code in ListController's onAttach() you will see that if add RecyclerView and immediately remove it, the app will not crash.

[EricKuck]

Your logging takes place from a subview, which isn't helpful. If you instead subclass the root LinearLayout, you'll see that the attach and detach events happen as you'd expect.

Also, I'm not getting a crash running your sample app. Which version of Android are you running it on?

[fedoretskn]

I've added RecyclerView in layout of ListController for reproducing crash.

So you think that if content views first receive onViewDetachedFromWindow event and after that onViewAttachedToWindow that's ok?

https://github.com/bluelinelabs/Conductor/blob/develop/conductor/src/main/java/com/bluelinelabs/conductor/Controller.java#L918
here you subscribe on attach changes in root view and you ignore that all subviews haven't received their events yet. But in any case I can rebuild layout in any time in controller's onAttach method before as subviews receive onViewAttachedToWindow event.

[EricKuck]

I don't understand why the detached event is being sent before the attached event, but that's not happening because of Conductor. This seems like an Android issue.

Regarding subview attach events, are you suggesting that this method recursively go through the entire hierarchy of the view's subviews and wait until they're all attached before doing anything?

[EricKuck]

Update: I was finally able to reproduce the issue you were originally reporting and am working on a fix. Watching the hierarchy of children actually might be needed, which sucks.

[EricKuck]

This should now be fixed in the latest snapshot if you want to give it a quick spin.

[fedoretskn]

Thank you!

[Ankur008]

@fedoretskn @EricKuck I am getting the same issue in my project. Can you help me with this issues?

[fedoretskn]

@Ankur008 As I understand, this issue is fixed. Could you kindly provide more details about your issue?

[Ankur008]

@fedoretskn Actually in my project( which is not related to Conductor project) getting the same issue, but I am unable to find the root cause of the problem? Can you help me why this problem occurs?

[abou7mied]

I have same issue with RecyclerView in project doesn't use Conductor, can you help us to know why this problem occurs?. Thanks