From 6a7a5698318b530e319e5058938cd634a10b58b2 Mon Sep 17 00:00:00 2001
From: Jean Chalard <jchalard@google.com>
Date: Wed, 10 Jul 2013 11:54:23 +0900
Subject: [PATCH] Serialize commands sent to a separate threads

Bug: 9715797
Change-Id: I1eda4d2f0056f70cfb8a92d658e0875706efc170
---
 .../dictionarypack/DictionaryService.java     | 59 +++++++++++--------
 1 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java b/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java
index 767f895dc..939c25f10 100644
--- a/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java
+++ b/java/src/com/android/inputmethod/dictionarypack/DictionaryService.java
@@ -22,13 +22,14 @@ import android.app.Service;
 import android.content.Context;
 import android.content.Intent;
 import android.os.IBinder;
-import android.util.Log;
 import android.widget.Toast;
 
 import com.android.inputmethod.latin.R;
 
 import java.util.Locale;
 import java.util.Random;
+import java.util.concurrent.LinkedBlockingQueue;
+import java.util.concurrent.ThreadPoolExecutor;
 import java.util.concurrent.TimeUnit;
 
 /**
@@ -92,21 +93,27 @@ public final class DictionaryService extends Service {
     private static final long VERY_LONG_TIME = TimeUnit.DAYS.toMillis(14);
 
     /**
-     * The last seen start Id. This must be stored because we must only call stopSelfResult() with
-     * the last seen Id, or the service won't stop.
+     * An executor that serializes tasks given to it.
      */
-    private int mLastSeenStartId;
-
-    /**
-     * The command count. We need this because we need to not call stopSelfResult() while we still
-     * have commands running.
-     */
-    private int mCommandCount;
+    private ThreadPoolExecutor mExecutor;
+    private static final int WORKER_THREAD_TIMEOUT_SECONDS = 15;
 
     @Override
     public void onCreate() {
-        mLastSeenStartId = 0;
-        mCommandCount = 0;
+        // By default, a thread pool executor does not timeout its core threads, so it will
+        // never kill them when there isn't any work to do any more. That would mean the service
+        // can never die! By creating it this way and calling allowCoreThreadTimeOut, we allow
+        // the single thread to time out after WORKER_THREAD_TIMEOUT_SECONDS = 15 seconds, allowing
+        // the process to be reclaimed by the system any time after that if it's not doing
+        // anything else.
+        // Executors#newSingleThreadExecutor creates a ThreadPoolExecutor but it returns the
+        // superclass ExecutorService which does not have the #allowCoreThreadTimeOut method,
+        // so we can't use that.
+        mExecutor = new ThreadPoolExecutor(1 /* corePoolSize */, 1 /* maximumPoolSize */,
+                WORKER_THREAD_TIMEOUT_SECONDS /* keepAliveTime */,
+                TimeUnit.SECONDS /* unit for keepAliveTime */,
+                new LinkedBlockingQueue<Runnable>() /* workQueue */);
+        mExecutor.allowCoreThreadTimeOut(true);
     }
 
     @Override
@@ -131,33 +138,35 @@ public final class DictionaryService extends Service {
      * - Handle a finished download.
      *     This executes the actions that must be taken after a file (metadata or dictionary data
      *     has been downloaded (or failed to download).
+     * The commands that can be spun an another thread will be executed serially, in order, on
+     * a worker thread that is created on demand and terminates after a short while if there isn't
+     * any work left to do.
      */
     @Override
     public synchronized int onStartCommand(final Intent intent, final int flags,
             final int startId) {
         final DictionaryService self = this;
-        mLastSeenStartId = startId;
-        mCommandCount += 1;
         if (SHOW_DOWNLOAD_TOAST_INTENT_ACTION.equals(intent.getAction())) {
             // This is a UI action, it can't be run in another thread
             showStartDownloadingToast(this, LocaleUtils.constructLocaleFromString(
                     intent.getStringExtra(LOCALE_INTENT_ARGUMENT)));
         } else {
-            // If it's a command that does not require UI, create a thread to do the work
-            // and return right away. DATE_CHANGED or UPDATE_NOW are examples of such commands.
-            new Thread("updateOrFinishDownload") {
+            // If it's a command that does not require UI, arrange for the work to be done on a
+            // separate thread, so that we can return right away. The executor will spawn a thread
+            // if necessary, or reuse a thread that has become idle as appropriate.
+            // DATE_CHANGED or UPDATE_NOW are examples of commands that can be done on another
+            // thread.
+            mExecutor.submit(new Runnable() {
                 @Override
                 public void run() {
                     dispatchBroadcast(self, intent);
-                    synchronized(self) {
-                        if (--mCommandCount <= 0) {
-                            if (!stopSelfResult(mLastSeenStartId)) {
-                                Log.e(TAG, "Can't stop ourselves");
-                            }
-                        }
-                    }
+                    // Since calls to onStartCommand are serialized, the submissions to the executor
+                    // are serialized. That means we are guaranteed to call the stopSelfResult()
+                    // in the same order that we got them, so we don't need to take care of the
+                    // order.
+                    stopSelfResult(startId);
                 }
-            }.start();
+            });
         }
         return Service.START_REDELIVER_INTENT;
     }