Work on issue #246

robospice-core-parent/robospice-core-test/src/main/java/com/octo/android/robospice/SpiceManagerTest.java

@@ -59,7 +59,7 @@ protected void tearDown() throws Exception {
         getContext().stopService(new Intent(getContext(), SpiceTestService.class));
         super.tearDown();
     }
-    
+
     private void waitForSpiceManagerShutdown(SpiceManagerUnderTest spiceManager) throws InterruptedException {
         if (spiceManager != null && spiceManager.isStarted()) {
             spiceManager.cancelAllRequests();
@@ -119,6 +119,22 @@ public void test_execute_should_fail_if_stopped() throws InterruptedException {
         }
     }
 
+    public void test_execute_should_execute_request_even_if_stopped_right_after_execute() throws InterruptedException {
+        // given
+        spiceManager.start(getContext());
+        SpiceRequestStub<String> spiceRequestStub = new SpiceRequestSucceedingStub<String>(TEST_CLASS, TEST_RETURNED_DATA);
+        RequestListenerStub<String> requestListenerStub = new RequestListenerStub<String>();
+
+        // when
+        spiceManager.execute(spiceRequestStub, TEST_CACHE_KEY, TEST_DURATION, requestListenerStub);
+        spiceManager.shouldStop();
+
+        spiceRequestStub.awaitForLoadDataFromNetworkIsCalled(REQUEST_COMPLETION_TIME_OUT);
+
+        // then
+        assertTrue(spiceRequestStub.isLoadDataFromNetworkCalled());
+    }
+
     public void test_execute_executes_1_request_that_succeeds() throws InterruptedException {
         // when
         spiceManager.start(getContext());
@@ -715,7 +731,7 @@ public void test_spice_managers_start_stop_many_times_quickly_kills_all_his_thre
             }
         }
 
-        spiceManager.shouldStopAndJoin(REQUEST_COMPLETION_TIME_OUT);
+        spiceManager.shouldStopAndJoin(2 * REQUEST_COMPLETION_TIME_OUT);
 
         // test
         // give some time for all threads to die of their most noble death

robospice-core-parent/robospice/src/main/java/com/octo/android/robospice/SpiceManager.java

@@ -86,6 +86,8 @@ public class SpiceManager implements Runnable {
     protected static final String SPICE_MANAGER_THREAD_NAM_PREFIX = "SpiceManagerThread ";
     /** Number of threads used to execute internal commands. */
     private static final int DEFAULT_THREAD_COUNT = 3;
+    /** Delay to let runner stop properly (in ms). */
+    private static final int DELAY_WAIT_FOR_RUNNER_TO_STOP = 500;
 
     // ============================================================================================
     // ATTRIBUTES
@@ -191,9 +193,9 @@ protected int getThreadCount() {
      * Start the {@link SpiceManager}. It will bind asynchronously to the
      * {@link SpiceService}.
      * @param context
-     *            a context that will be used to bind to the
-     *            service. Typically, the Activity or Fragment that needs to
-     *            interact with the {@link SpiceService}.
+     *            a context that will be used to bind to the service. Typically,
+     *            the Activity or Fragment that needs to interact with the
+     *            {@link SpiceService}.
      */
     public synchronized void start(final Context context) {
         this.contextWeakReference = new WeakReference<Context>(context);
@@ -272,9 +274,13 @@ private void sendRequestToService(final CachedSpiceRequest<?> spiceRequest) {
         lockSendRequestsToService.lock();
         try {
             if (spiceRequest != null && spiceService != null) {
-                final Set<RequestListener<?>> listRequestListener = mapRequestToLaunchToRequestListener.get(spiceRequest);
-                Ln.d("Sending request to service : " + spiceRequest.getClass().getSimpleName());
-                spiceService.addRequest(spiceRequest, listRequestListener);
+                if (isStopped) {
+                    spiceService.addRequest(spiceRequest, null);
+                } else {
+                    final Set<RequestListener<?>> listRequestListener = mapRequestToLaunchToRequestListener.get(spiceRequest);
+                    Ln.d("Sending request to service : " + spiceRequest.getClass().getSimpleName());
+                    spiceService.addRequest(spiceRequest, listRequestListener);
+                }
             }
         } finally {
             lockSendRequestsToService.unlock();
@@ -289,20 +295,11 @@ private void sendRequestToService(final CachedSpiceRequest<?> spiceRequest) {
      * {@link SpiceRequest}s. Unbinding will occur asynchronously.
      */
     public synchronized void shouldStop() {
-        if (!isStarted()) {
-            throw new IllegalStateException("Not started yet");
+        try {
+            shouldStopAndJoin(DELAY_WAIT_FOR_RUNNER_TO_STOP);
+        } catch (InterruptedException e) {
+            Ln.e(e, "Exception when joining the runner that was stopping.");
         }
-        Ln.d("SpiceManager stopping.");
-        dontNotifyAnyRequestListenersInternal();
-        isUnbinding = false;
-        unbindFromService(contextWeakReference.get());
-        spiceServiceConnection = null;
-        this.isStopped = true;
-        this.runner.interrupt();
-        this.runner = null;
-        this.executorService.shutdown();
-        this.contextWeakReference.clear();
-        Ln.d("SpiceManager stopped.");
     }
 
     /**
@@ -320,11 +317,22 @@ public synchronized void shouldStopAndJoin(final long timeOut) throws Interrupte
         }
 
         Ln.d("SpiceManager stopping. Joining");
+        this.isStopped = true;
         dontNotifyAnyRequestListenersInternal();
+        if (requestQueue.isEmpty()) {
+            this.runner.interrupt();
+        }
+        long start = System.currentTimeMillis();
+        try {
+            this.runner.join(timeOut);
+        } catch (InterruptedException e) {
+            throw e;
+        } finally {
+            long end = System.currentTimeMillis();
+            Ln.d("Runner join time (ms) when should stop %d", (end - start));
+        }
+        isUnbinding = false;
         unbindFromService(contextWeakReference.get());
-        this.isStopped = true;
-        this.runner.interrupt();
-        this.runner.join(timeOut);
         this.runner = null;
         this.executorService.shutdown();
         this.contextWeakReference.clear();
@@ -472,6 +480,7 @@ public <T> void execute(final SpiceRequest<T> request, final Object requestCache
      */
     public <T> void execute(final CachedSpiceRequest<T> cachedSpiceRequest, final RequestListener<T> requestListener) {
         addRequestListenerToListOfRequestListeners(cachedSpiceRequest, requestListener);
+        System.out.println("adding request to request queue");
         this.requestQueue.add(cachedSpiceRequest);
     }
 
@@ -877,15 +886,16 @@ public <T> Future<List<T>> getAllDataFromCache(final Class<T> clazz) throws Cach
      * This method doesn't perform any network processing, it just checks if
      * there are previously saved data. Don't call this method in the main
      * thread because you could block it. Instead, use the asynchronous version
-     * of this method: {@link #getFromCache(Class, Object, long, RequestListener)}.
+     * of this method:
+     * {@link #getFromCache(Class, Object, long, RequestListener)}.
      * @param clazz
      *            the class of the result to retrieve from cache.
      * @param cacheKey
      *            the key used to store and retrieve the result of the request
      *            in the cache
      * @return a future object that will hold data in cache. Calling get on this
-     *  future will block until the data is actually effectively taken from cache. 
-     * 
+     *         future will block until the data is actually effectively taken
+     *         from cache.
      * @throws CacheLoadingException
      *             Exception thrown when a problem occurs while loading data
      *             from cache.
@@ -899,8 +909,8 @@ public <T> Future<T> getDataFromCache(final Class<T> clazz, final Object cacheKe
      * method doesn't perform any network processing, it just data in cache,
      * erasing any previsouly saved date in cache using the same class and key.
      * Don't call this method in the main thread because you could block it.
-     * Instead, use the asynchronous version of this method: {@link
-     * #putInCache(Class, Object, Object)}.
+     * Instead, use the asynchronous version of this method:
+     * {@link #putInCache(Class, Object, Object)}.
      * @param cacheKey
      *            the key used to store and retrieve the result of the request
      *            in the cache
@@ -1158,8 +1168,9 @@ private boolean tryToStartService() {
     }
 
     private void bindToService(final Context context) {
-        if (context == null || isStopped) {
+        if (context == null || (requestQueue.isEmpty() && isStopped)) {
             // fix issue 40. Thx Shussu
+            // fix issue 246.
             return;
         }