Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IllegalStateException when you try to change the icon #21

Closed
btimofeev opened this issue Jan 20, 2016 · 7 comments

Comments

@btimofeev
Copy link
Contributor

commented Jan 20, 2016

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)

@y20k y20k added the bug label Jan 21, 2016

@y20k y20k self-assigned this Jan 21, 2016

@y20k

This comment has been minimized.

Copy link
Owner

commented Jan 21, 2016

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.

y20k added a commit that referenced this issue Jan 22, 2016

Possible fix for the stupid IllegalStateException bug (issue #21) tha…
…t I am hunting since December. Needs testing.
@btimofeev

This comment has been minimized.

Copy link
Contributor Author

commented Jan 22, 2016

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.

@y20k

This comment has been minimized.

Copy link
Owner

commented Jan 22, 2016

Okay. That is not good. But a least no crash like before, right?

I'll look into it. Thank you for testing this.

@y20k

This comment has been minimized.

Copy link
Owner

commented Feb 1, 2016

moved this issue into a new separate bug report (#34)

@y20k y20k closed this Feb 1, 2016

@FAndSec

This comment has been minimized.

Copy link

commented Jan 8, 2019

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().

@y20k

This comment has been minimized.

Copy link
Owner

commented Jan 9, 2019

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()?

@FAndSec

This comment has been minimized.

Copy link

commented Jan 9, 2019

Hello,

  1. 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).

  1. No. onPause and onStop will always be called prior to onDestroy being called. The Fragment lifecycle will be pushed through without missing any callback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.