better protect against race condition with onStartJob/onStopJob with …

…synchronized
AltBeacon · Jul 5, 2019 · 4d58053 · 4d58053
1 parent d7b5d2d
commit 4d58053
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 55 deletions.
diff --git a/build.gradle b/build.gradle
@@ -5,7 +5,7 @@ buildscript {
     }
 
     dependencies {
-        classpath 'com.android.tools.build:gradle:3.2.1'
+        classpath 'com.android.tools.build:gradle:3.4.1'
         //noinspection GradleDependency
         classpath 'com.jfrog.bintray.gradle:gradle-bintray-plugin:1.8.1'
         //noinspection GradleDependency

diff --git a/lib/src/main/java/org/altbeacon/beacon/service/ScanJob.java b/lib/src/main/java/org/altbeacon/beacon/service/ScanJob.java
@@ -88,52 +88,58 @@ public void run() {
                 }
                 LogManager.d(TAG, "Done processing queued scan resuilts");
 
-                if (mStopCalled) {
-                    LogManager.d(TAG, "Quitting scan job before we even start.  Somebody told us to stop.");
-                    ScanJob.this.jobFinished(jobParameters , false);
-                    return;
-                }
+                // This syncronized block is around the scan start.
+                // Without it, it is possilbe that onStopJob is called in another thread and
+                // closing out the CycledScanner
+                synchronized(this) {
+                    if (mStopCalled) {
+                        LogManager.d(TAG, "Quitting scan job before we even start.  Somebody told us to stop.");
+                        ScanJob.this.jobFinished(jobParameters , false);
+                        return;
+                    }
 
-                boolean startedScan;
-                if (mInitialized) {
-                    LogManager.d(TAG, "Scanning already started.  Resetting for current parameters");
-                    startedScan = restartScanning();
-                }
-                else {
-                    startedScan = startScanning();
-                }
-                mStopHandler.removeCallbacksAndMessages(null);
+                    boolean startedScan;
+                    if (mInitialized) {
+                        LogManager.d(TAG, "Scanning already started.  Resetting for current parameters");
+                        startedScan = restartScanning();
+                    }
+                    else {
+                        startedScan = startScanning();
+                    }
+                    mStopHandler.removeCallbacksAndMessages(null);
 
-                if (startedScan) {
-                    if (mScanState != null) {
-                        LogManager.i(TAG, "Scan job running for "+mScanState.getScanJobRuntimeMillis()+" millis");
-                        mStopHandler.postDelayed(new Runnable() {
-                            @Override
-                            public void run() {
-                                LogManager.i(TAG, "Scan job runtime expired: " + ScanJob.this);
-                                stopScanning();
-                                mScanState.save();
-                                ScanJob.this.jobFinished(jobParameters , false);
+                    if (startedScan) {
+                        if (mScanState != null) {
+                            LogManager.i(TAG, "Scan job running for "+mScanState.getScanJobRuntimeMillis()+" millis");
+                            mStopHandler.postDelayed(new Runnable() {
+                                @Override
+                                public void run() {
+                                    LogManager.i(TAG, "Scan job runtime expired: " + ScanJob.this);
+                                    stopScanning();
+                                    mScanState.save();
+                                    ScanJob.this.jobFinished(jobParameters , false);
 
-                                // need to execute this after the current block or Android stops this job prematurely
-                                mStopHandler.post(new Runnable() {
-                                    @Override
-                                    public void run() {
-                                        scheduleNextScan();
-                                    }
-                                });
+                                    // need to execute this after the current block or Android stops this job prematurely
+                                    mStopHandler.post(new Runnable() {
+                                        @Override
+                                        public void run() {
+                                            scheduleNextScan();
+                                        }
+                                    });
 
-                            }
-                        }, mScanState.getScanJobRuntimeMillis());
+                                }
+                            }, mScanState.getScanJobRuntimeMillis());
+                        }
+                    }
+                    else {
+                        LogManager.i(TAG, "Scanning not started so Scan job is complete.");
+                        stopScanning();
+                        mScanState.save();
+                        LogManager.d(TAG, "ScanJob Lifecycle STOP (start fail): "+ScanJob.this);
+                        ScanJob.this.jobFinished(jobParameters , false);
                     }
                 }
-                else {
-                    LogManager.i(TAG, "Scanning not started so Scan job is complete.");
-                    stopScanning();
-                    mScanState.save();
-                    LogManager.d(TAG, "ScanJob Lifecycle STOP (start fail): "+ScanJob.this);
-                    ScanJob.this.jobFinished(jobParameters , false);
-                }
+
             }
         }).start();
 
@@ -183,21 +189,24 @@ private void startPassiveScanIfNeeded() {
 
     @Override
     public boolean onStopJob(JobParameters params) {
-        mStopCalled = true;
-        if (params.getJobId() == getPeriodicScanJobId(this)) {
-            LogManager.i(TAG, "onStopJob called for periodic scan " + this);
-        }
-        else {
-            LogManager.i(TAG, "onStopJob called for immediate scan " + this);
-        }
-        LogManager.d(TAG, "ScanJob Lifecycle STOP: "+ScanJob.this);
-        // Cancel the stop timer.  The OS is stopping prematurely
-        mStopHandler.removeCallbacksAndMessages(null);
+        // See corresponding synchronized block in onStartJob
+        synchronized(this) {
+            mStopCalled = true;
+            if (params.getJobId() == getPeriodicScanJobId(this)) {
+                LogManager.i(TAG, "onStopJob called for periodic scan " + this);
+            }
+            else {
+                LogManager.i(TAG, "onStopJob called for immediate scan " + this);
+            }
+            LogManager.d(TAG, "ScanJob Lifecycle STOP: "+ScanJob.this);
+            // Cancel the stop timer.  The OS is stopping prematurely
+            mStopHandler.removeCallbacksAndMessages(null);
 
-        stopScanning();
-        startPassiveScanIfNeeded();
-        if (mScanHelper != null) {
-            mScanHelper.terminateThreads();
+            stopScanning();
+            startPassiveScanIfNeeded();
+            if (mScanHelper != null) {
+                mScanHelper.terminateThreads();
+            }
         }
         return false;
     }