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

fix IllegalArgumentException from dismiss #135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ftc
Copy link

@ftc ftc commented Mar 18, 2018

Hi I am with the University of Colorado programming languages and verification group. We are working on methods to automatically detect hard to find defects in Android applications. I recently found this crash with the tools I am developing for my research and attempted to fix it to the best of my abilities.

To reproduce:

  • You should be able to use any device, however I tested on the emulator and my Samsung galaxy s7
  • Create a track
  • Export or upload track and quickly rotate phone
  • Application will crash with an IllegalArgumentException if the timing right
  • To make the crash easier to observe in testing, delay the doInBackground method of ExportTrackTask by half a second, however this is not necessary as a slow SD card or network connection could cause this.
        @Override
        protected Boolean doInBackground(Void... params) {
                try {
                        Thread.sleep(500);
                } catch (InterruptedException e) {
                        e.printStackTrace();
                }

ADB logcat / stacktrace

E/AndroidRuntime: FATAL EXCEPTION: main
                  Process: org.osmtracker, PID: 4182
                  java.lang.IllegalArgumentException: View=DecorView@5bd7f9f[] not attached to window manager
                      at android.view.WindowManagerGlobal.findViewLocked(WindowManagerGlobal.java:473)
                      at android.view.WindowManagerGlobal.removeView(WindowManagerGlobal.java:382)
                      at android.view.WindowManagerImpl.removeViewImmediate(WindowManagerImpl.java:124)
                      at android.app.Dialog.dismissDialog(Dialog.java:357)
                      at android.app.Dialog.dismiss(Dialog.java:340)
                      at org.osmtracker.gpx.ExportTrackTask.onProgressUpdate(ExportTrackTask.java:160)
                      at org.osmtracker.gpx.ExportTrackTask.onProgressUpdate(ExportTrackTask.java:42)
                      at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:680)
                      at android.os.Handler.dispatchMessage(Handler.java:102)
                      at android.os.Looper.loop(Looper.java:154)
                      at android.app.ActivityThread.main(ActivityThread.java:6077)
                      at java.lang.reflect.Method.invoke(Native Method)
                      at com.android.internal.os.ZygoteInit$MethodAndArgsCaller.run(ZygoteInit.java:865)
                      at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:755)

Explanation

The dismiss() method on ProgressDialog`` cannot be called after the associated Activityobject has been paused. Since there are dismiss methods in thedoInBackground()andonPostExecute()``` callbacks it is possible for these to be invoked after the activity has paused but before it has resumed.

Fix

To fix this I dismiss the dialog when the activity is paused and then check if it is showing before dismiss is invoked. Unfortunately the isShowing() method will return true after the dialog has been hidden through a rotation so the dismiss() in the onPause() callback is necessary.

I hope this helps! Great application by the way.

@xsaco07
Copy link
Contributor

xsaco07 commented Aug 23, 2018

Hi, thanks for contributing!
A few weeks ago we made some improvements in the develop and master branch. This pull request has produced some conflicts when I try to test it because the origin branches have been updated. I've tried to solve them but up to this moment I have not succeeded. I will continue trying to fix that but the better solution we found is that you update the origin branches and make this improvement from there.

Specifically start from the develop branch -> make your changes -> make pull request.

Thank you very much and we hope you understand.

@ftc
Copy link
Author

ftc commented Sep 5, 2018

Sure no problem, it may be a couple days before I can get my changes applied but I am happy to do it.

@xsaco07
Copy link
Contributor

xsaco07 commented Sep 11, 2018

Excellent! Thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants