IllegalStateException when you try to change the icon
btimofeev opened this issue · 7 comments
How to reproduce the error:
open Transistor -> shut down by pressing the back -> open again -> try to change the icon of any station.
01-21 00:37:46.789 31054-31054/org.y20k.transistor E/AndroidRuntime: FATAL EXCEPTION: main
Process: org.y20k.transistor, PID: 31054
java.lang.IllegalStateException: Fragment MainActivityFragment{e7db358} not attached to Activity
at android.support.v4.app.Fragment.startActivityForResult(Fragment.java:925)
at org.y20k.transistor.MainActivityFragment.selectFromImagePicker(MainActivityFragment.java:482)
at org.y20k.transistor.MainActivityFragment.access$500(MainActivityFragment.java:58)
at org.y20k.transistor.MainActivityFragment$6.onReceive(MainActivityFragment.java:415)
at android.support.v4.content.LocalBroadcastManager.executePendingBroadcasts(LocalBroadcastManager.java:297)
at android.support.v4.content.LocalBroadcastManager.access$000(LocalBroadcastManager.java:46)
at android.support.v4.content.LocalBroadcastManager$1.handleMessage(LocalBroadcastManager.java:116)
at android.os.Handler.dispatchMessage(Handler.java:102)
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)
Thank you for the bug report. Especially for the description how to reproduce it! I experienced this bug several times during development of the "add image" feature. But I was never able to reproduce it. Now I can test out several possible solutions.
I did dig into the problem before. I thought the problem is that the parent activity and the child fragment get detached on quitting the app. When restarting the app the fragment gets attached to a new activity. That somehow causes the IllegalStateException because the fragment maybe still holding references to the old activity it was attached to in the first place.
I will look into it. This bug is bugging me. But please be patient. I am operating on a small time budget. I have a full time job and a family. Progress may be a bit slow here.
Commit ec0b923 added a new error:
open Transistor -> tap to change icon -> press back -> press back -> open Transistor -> tap to change icon -> press back -> press back
The app is not closed, and opens a window for selecting images.
Okay. That is not good. But a least no crash like before, right?
I'll look into it. Thank you for testing this.
Hello, I know you have closed this issue but we have a solid fix for this bug, and also the bug in @btimofeev 's comment. Note that you registered a BroadcastReceiver to listen for a self-defined action ACTION_IMAGE_CHANGE_REQUESTED in the context of Application. When user presses the back button in the resumed state of your MainActivity. The onDestroy callback of the activity is called, so does the MainActivityFragment. But the context of your Application is not. Since now the Application context still lives, the aforementioned BroadcastReceiver is still registered. However, on re-instantiating your MainActivity by tapping your app icon, the onCreate(Bundle) callback is called again, this is where you register your BroadcastReceiver. So now you have two instances of this BroadcastReceiver both registered and listening to ACTION_IMAGE_CHANGE_REQUESTED, note that the new one is good to go, however the old one holds reference to the Fragment (because in its onReceive callback, it calls a private method selectFromImagePicker on the Fragment, who in turn references to its containing MainActivity with private field member mActivity. (Note that although the onDestroy callback is called on both the old Fragment and the old Activity, they are never garbage collected, because they are still referenced). Now when the action is issued, both receiver responds, it is the old one that throws the IllegalStateException. How? Because the old Fragment is not attached to the old Activity, although you defined mActivity to refer to it, it is called referring, not attaching. Actually, Fragment has a package private field mHost to attach to its containing Activity. Now mHost is null in old Fragment, IllegalStateException is thrown. For @btimofeev 's comment, remember that the old receiver wants to execute startActivityForResult(...) on mActivity, recall that it still holds reference to the old MainActivity object, thus a new activity for image selection is started, let's call it image selection activity No. 1, and soon pushed to the back stack of Gallery app (or any app that opens the image selection activity for you depending on device) because the new BroadcastReceiver wants to create its own new activity, let's call it image selection activity No. 2, on successfully creating new activity, MainActivity No. 2 is pushed to the Transistor's back stack. On destroying image selection activity NO. 2 by pressing back button, the MainActivity No. 2 pops from Transistor's back stack. On pressing back button one more time, since the back stack for Transistor is empty, now the Gallery app pops the image selection activity No. 1. One way to verify with your own eyes is that at this moment, you can click the overview button and will see the overview for both MainActivity No. 2 and image selection activity No. 1 respectively, since there are two back stacks.
To fix: unregister your BroadcastReceiver in onDestroy().
Hi @FAndSec ,
thanks for the explanation! That seems very plausible.
In the current version of Transistor there is no need for a BroadcastReceiver listening for image change requests from other parts of the app. So it was removed in the past.
I haven't had a look into this isssue for a while. So I am not sure if it still is a problem. If so, the BroadcastReceiver cannot be the cause, since it not present anymore.
Anyway. This still is very helpful for me to know going forward. Just to clarify... In general: To just simply unregister a receiver in ... lets say onStop() or onPause() and not in in onDestroy() can cause problems like this? Does the call to onDestroy() skip other steps in the lifecycle like onPause()?
Hello,
- Unregistering receiver in onStop or onPause raise new bugs. The second time the user wants to change icon, the image selection Activity won't show.
reason: onPause and onStop of MainActivityFragment will be called in sequence when user leaves current activity, so first time user change icon, app works fine. However, since receiver is already unregistered, second time trying to change icon will not open new image selection activity.
I have tested this on LG Nexus 5X (also for the previous bugs).
- No. onPause and onStop will always be called prior to onDestroy being called. The Fragment lifecycle will be pushed through without missing any callback.