So you're smart. You know not to block the main (aka UI) thread in your Android app. So you put your heavy work in a background thread. Excellent.
You're also kind enough to display a progress dialog, since the user can't do much in your app until your work is done.
And you're even educated enough to know that you really should call dismissDialog on the UI thread.
But then you do something dumb, like I did. Your solution looks like mine (below).
It works great at first. The dialog pops up, the thread does its work, and 5 seconds later, the dialog goes away.
public class ScratchActivity extends Activity {
private static final String TAG = "ScratchActivity";
private ProgressDialog dialog = null;
@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.main);
Button button = (Button) findViewById(R.id.button);
button.setOnClickListener(new View.OnClickListener() {
@Override
public void onClick(View view) {
doSomethingInBackground();
}
});
}
protected void doSomethingInBackground() {
showDialog(1);
Thread someBackgroundProcess = new Thread() {
@Override
public void run() {
try {
Thread.sleep(5000);
allDoneNow();
} catch (Exception ex) {
Log.d(TAG, "ahhhh", ex);
}
}
};
someBackgroundProcess.start();
}
private void allDoneNow() {
runOnUiThread(new Runnable() {
@Override
public void run() {
try {
dismissDialog(1);
}catch(Exception ex) {
Log.d(TAG, "ahhhh", ex);
}
}
});
}
@Override
protected Dialog onCreateDialog(int id) {
Log.d(TAG, "onCreateDialog " + id);
if (dialog == null) {
dialog = new ProgressDialog(this);
}
return dialog;
}
Trouble Strikes
Then you do the unthinkable. You press the button to show the dialog, and before the dialog gets dismissed, you rotate your device into landscape mode.You wait. And wait. And wait...
The stupid black dialog never goes away. So you smartly check LogCat and discover:
no dialog with id 1 was ever shown !!
10-25 11:26:41.738: DEBUG/ScratchActivity(2279): ahhhh
10-25 11:26:41.738: DEBUG/ScratchActivity(2279): java.lang.IllegalArgumentException: no dialog with id 1 was ever shown via Activity#showDialog
10-25 11:26:41.738: DEBUG/ScratchActivity(2279): at android.app.Activity.missingDialog(Activity.java:2600)
10-25 11:26:41.738: DEBUG/ScratchActivity(2279): at android.app.Activity.dismissDialog(Activity.java:2585)
10-25 11:26:41.738: DEBUG/ScratchActivity(2279): at com.scratch.ScratchActivity$3.run(ScratchActivity.java:68)
"It sure the heck was shown!" You scream at the stupid framework. "If it wasn't shown, how is it on my screen?"
Idiotic voodoo framework.
Oh, oops
After a little noodling on the problem for a while -- even wondering whether there is a bug in the framework -- you (by which I mean me) realize the problem is indeed your own stupid code, by which I mean yours, not mine ;-)
The problem is subtle, insofar as it involves not what is explicit in the code, but what is implicit in the code. When you create your anonymous inner class on line 26 above --
... new Thread() {
...
allDoneNow();
...
What ScratchActivity instance is allDoneNow going to execute with? The anonymous class gets an implicit reference to the current ScratchActivity instance. Ok, fine.
Problem is, when you change orientation, Android -- by default -- destroys your activity and creates a new one. That means the anonymous inner class is now referring to the old ScratchActivity instance (which is now torn-down). The activity on screen is a new instance.
Now, if you use showDialog to create a dialog, the Activity class adds the dialog to a list of dialogs that it is managing (so that it can dismiss it, restore it, etc. during the activity life cycle).
Later, in Activity.onDestroy, the list of so-called "managed dialogs" gets set to null. So it makes sense, then, that calling dismissDialog on a destroyed activity must fail.
Fabulous! So, let's change our code to ensure that the background thread will call the right ScratchActivity instance.
One thing we have to consider, though, is a kind of race condition. We have to ensure that we do not try to fetch a reference to the activity while a configuration change is impending. [Orientation changes are considered configuration changes.]
The simplest way to do that is by having the code execute on the UI thread. Fortunately, we've already done that by using Activity.runOnUiThread.
Once on the UI thread, it is guaranteed that the call to dismissDialog will occur entirely before or entirely after the orientation change.
Now that we've got that covered, the most straightforward thing to do is add a static field to hold the currently active Activity. Then we set that field in onCreate.
private static Activity currentActivity = null;
public static synchronized Activity getCurrentActivity() {
return currentActivity;
}
public static synchronized void setCurrentActivity(Activity activity) {
currentActivity = activity;
}
@Override
public void onCreate(Bundle savedInstanceState) {
setCurrentActivity(this);
super.onCreate(savedInstanceState);
Then, allDoneNow, instead of calling just dismissDialog(1); calls getCurrentActivity().dismissDialog(1);
Problem solved, right?
Ugh, Not Yet
This code is bad too! We just caused a memory leak. When the activity is all done -- for real, not because of an orientation change -- currentActivity (a static field) continues to hold a reference to the activity, which keeps it, and the entire object graph that it references, in memory.
Simple, fix, though. Just be sure to call setCurrentActivity(null) in onDestroy.
So an acceptable solution might look like this. Although, there are other ways to do this, and it sure would be nice if the framework helped a little more. Maybe Activity itself could provide this capability.
public class ScratchActivity extends Activity {
private static final String TAG = "ScratchActivity";
private ProgressDialog dialog = null;
private static Activity currentActivity = null;
public static synchronized Activity getCurrentActivity() {
return currentActivity;
}
public static synchronized void setCurrentActivity(Activity activity) {
currentActivity = activity;
}
@Override
protected void onDestroy() {
super.onDestroy();
setCurrentActivity(null);
}
@Override
public void onCreate(Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setContentView(R.layout.main);
Button button = (Button) findViewById(R.id.button);
button.setOnClickListener(new View.OnClickListener() {
@Override
public void onClick(View view) {
doSomethingInBackground();
}
});
}
protected void doSomethingInBackground() {
showDialog(1);
Thread someBackgroundProcess = new Thread() {
@Override
public void run() {
try {
Thread.sleep(5000);
allDoneNow();
} catch (Exception ex) {
Log.d(TAG, "ahhhh", ex);
}
}
};
someBackgroundProcess.start();
}
private void allDoneNow() {
runOnUiThread(new Runnable() {
@Override
public void run() {
try {
dismissDialog(1);
}catch(Exception ex) {
Log.d(TAG, "ahhhh", ex);
}
}
});
}
@Override
protected Dialog onCreateDialog(int id) {
Log.d(TAG, "onCreateDialog " + id);
if (dialog == null) {
dialog = new ProgressDialog(this);
}
return dialog;
}
There are other solutions to this problem of course, but this one is simple enough for now.All of this brings me to the topic of my next post:
Are Anonymous Inner Classes Evil for Android Apps?
No comments:
Post a Comment