3

This is an extension of my question here, where you can see all of my code for both the fragment containing the button and the Service being used: Android sending messages between fragment and service

I have a button, which when clicked will start a Service, collect some sensor data, insert into database. When clicked again, I want to display a progress dialog, wait for existing background tasks to finish and then dismiss the progress dialog.

I can now correctly send messages back and forth between the fragment and service using a Handler in the fragment (as can be seen in my other question). The problem is that the progress dialog doesn't show in the UI thread until AFTER the executor task queue has been cleared

In my fragment, the button onClick looks like this:

//Show stop dialog
stopDialog = new ProgressDialog(mainActivity);
stopDialog.setMessage("Stopping...");
stopDialog.setTitle("Saving data");
stopDialog.setProgressNumberFormat(null);
stopDialog.setCancelable(false);
stopDialog.setMax(100);
stopDialog.show();
Log.d(TAG, "dialog up");

//Stop the service
mainActivity.stopService(new Intent(mainActivity, SensorService.class));

and my Service onDestroy looks like this:

public void onDestroy() {
    //Prevent new tasks from being added to thread
    executor.shutdown();
    Log.d(TAG, "Executor shutdown is called");

    try {
        //Wait for all tasks to finish before we proceed
        while (!executor.awaitTermination(1, TimeUnit.SECONDS)) {
            Log.i(TAG, "Waiting for current tasks to finish");
        }
    } catch (InterruptedException e) {
        executor.shutdownNow();
        Thread.currentThread().interrupt();
    }

    if (executor.isTerminated()){
        //Stop everything else once the task queue is clear
        unregisterReceiver(receiver);
        unregisterListener();
        wakeLock.release();
        dbHelper.close();
        stopForeground(true);

        //Dismiss progress dialog - commented out for debugging
        //sendMessage("HIDE");
    }
}

Expectation

When the Stop button is clicked, the progress dialog should show, onDestroy gets called in the service, the executor queue finishes existing tasks, and then dialog gets dismissed via a message to a handler (this last part has been commented out so I can figure out what is going on)

Reality

When Stop is clicked, in logcat I can see the message dialog up, followed by Executor shutdown is called. While its finishing the current task queue I can see Waiting for current tasks to finish. So the order of events is correct, but the progress dialog doesn't actually display until after all the onDestroy code, even though (in the fragment) I tell it to show before the service is even told to stop.

I also see a lot of skipped frames after I click the Stop button, presumably while the executor is trying to clear its queue: Skipped 336 frames! The application may be doing too much work on its main thread. Though I'm not sure why an executor working in a background thread is causing the UI to not update

Whats going on? UI blocking?

In my mind the dialog should show, and THEN service is told to stop, where it will work through the rest of the executor task queue, and then get dismissed. But for some reason the dialog show isn't happening until after the executor completely terminates.

Is there something in my code thats blocking the UI thread from showing the progress dialog? I would have thought that because the executor is running in a background thread that the UI thread will be free to show/display things like dialogs

Note: I know I can easily solve this using an AsyncTask, and theres a similar problem/solution to it here, but I want to use an executor for this. Most questions deal with activities and asynctasks, but my question uses a strange combination of fragments, services and executors, for which I cannot find any questions/answers

EDIT:

After some googling I found this question here, which suggests that awaitTermination might be blocking the UI thread. So I took most of my onDestroy code that waits for the executor queue to clear and moved it into a new thread:

public void onDestroy() {
    //Prevent new tasks from being added to thread
    executor.shutdown();
    Log.d(TAG, "Executor shutdown is called");

    new Thread(new Runnable() {

        public void run() {
            try {
                //Wait for all tasks to finish before we proceed
                while (!executor.awaitTermination(1, TimeUnit.SECONDS)) {
                    Log.i(TAG, "Waiting for current tasks to finish");
                }
            } catch (InterruptedException e) {
                executor.shutdownNow();
                Thread.currentThread().interrupt();
            }

            if (executor.isTerminated()) {
                //Stop everything else once the task queue is clear
                unregisterReceiver(receiver);
                unregisterListener();
                wakeLock.release();
                dbHelper.close();
                stopForeground(true);

                //Dismiss progress dialog
                sendMessage("HIDE");
            }
        }
    }).start();
}

Now when I hit Stop, it seems like the progress dialog shows instantly, but I also get this exception every time:

Exception dispatching input event. JNI DETECTED ERROR IN APPLICATION: JNI CallObjectMethod called with pending exception java.util.concurrent.RejectedExecutionException: Task com.example.app.SensorService$InsertHandler@35e04d3 rejected from java.util.concurrent.ThreadPoolExecutor@2b28810[Shutting down, pool size = 1, active threads = 1, queued tasks = 481, completed tasks = 2069]

(that is just the first part of the exception - it is very long)

I'm not sure what is happening. While that new thread is starting, the executor thread is running execute to finish the tasks in its current queue. Is this causing a problem in how the 2 threads are working concurrently?

Community
  • 1
  • 1
Simon
  • 9,762
  • 15
  • 62
  • 119

1 Answers1

2

You have correctly identified that the reason the Dialog was not showing up was because your UI thread was blocked in awaitTermination.

The reason for the RejectedExecutionException is that you are submitting tasks for execution after the ThreadPoolExecutor is already in shutting down state.

Note that the executor enters shutdown as soon as you call executor.shutdown(). However, you don't actually unregister your listener with the SensorManager until the executor is drained of all pending tasks. During that time you could be receiving onSensorChanged callbacks and thus calling executor.execute(...), each of which will result in the exception.

The simple workaround would be to check if executor.isShutdown() before calling execute(...), but it is more appropriate to unregister your listener from the SensorManager before shutting down the executor:

public class SensorService extends Service implements SensorEventListener {
    // ....

    public void onDestroy() {
        // Prevent any new tasks from being created.
        unregisterListener();
        // Shutdown 
        executor.shutdown();
        // ...
    }
}
szym
  • 5,606
  • 28
  • 34
  • But I dont really see where/how I'm submitting new tasks. The code for task submission (in my other question listed) is in `onSensorChanged` - every time a sensor event comes in I call `executor.execute`. But as soon as I hit the button, `onDestroy` should be called, calling `executor.shutdown`, at which point no new tasks should be allowed to be added. This is prior to creating the new thread for `awaitTermination`. So by the time the new thread starts, `shutdown` has been called which by itself should prevent new task submission – Simon Apr 23 '16 at 23:10
  • Furthermore, if it its true that tasks are still being submitted after `shutdown`, then this should have been a problem with my old code as well. Or does the new thread somehow cause additional tasks to be inserted? – Simon Apr 23 '16 at 23:10
  • You should include some of the code for registering the listener at the `SensorManager` (so that it the threading of the callbacks is apparent). I updated my answer to explain the timing issue you are likely observing. – szym Apr 24 '16 at 10:04
  • That seems to do it! Just to confirm before I accept the answer: I need to make sure that I dont miss any sensor events. So when Stop is pressed, there should still be a bunch of submitted tasks in the queue, and with this change they will all still get saved to the DB (i.e. queue gets fully cleared)? even though I'm unregistering the listener before telling the executor to shutdown? – Simon Apr 24 '16 at 11:59
  • Unregistering the listener will ensure you won't create any new tasks, but will not affect the tasks you already submitted to the executor. – szym Apr 24 '16 at 22:59