Skip to content

Commit 39a98f4

Browse files
committed
Start documenting thread usage for service objects
This makes the current implicit assumption that the scan cycler is started/stopped on the main thread explicit. This helps quickly inform developers what the callback expectations are in regards to race conditions and state safety. In order to verify this we need to trace the `start`/`stop`/`destroy` calls back to the service. The service's core methods `onCreate` and `onDestroy` are called on the main thread (see links below). This means the service creates it's `mMessenger` on the main thread, which creates the `IncomingHandler` on the main thread which binds to the main looper. However, the `IncomingHandler` class itself leaves the looper unspecified relying on looper for the thread which creates it. As we did in #430, this modifies `IncomingHandler` to explicitly use the main looper as a future proofing mechanism. This way we can ensure this implicit expectation doesn't change or at least make it obvious when troubleshooting cases where someone expects it to have changed. Similarly, this adds the main thread annotation to it's `handleMessage` implementation. Working from here we can confirm that the only places where the beacon monitoring/ranging is updated is from the main thread through the `IncomingHandler`. As the `IncomingHandler` is the main communication channel for the service we transfer the main thread expectation to the associated ranging/monitoring methods. _If_ someone decides to call these methods directly on the service these annotations help the IDEs check/document that such calls are expected from the main thread. With all of that documented we can now confidently state that the scan cycler's `start`, `stop`, and `destroy` are also meant to be called from the main thread. As `start` and `stop` both call `scanLeDevice` we can start tracing any other threading expectations for it. We already know it's called from the main thread through deferred jobs. This leaves the `finishScanCycle` as the last call site. `finishScanCycle` is only called from `scheduleScanCycleStop`. As this method name implies it's called through a deferred job on the main thread. It is also called the "first" time in `scanLeDevice`. Thus we've shown that `scanLeDevice`, `finishScanCycle`, and `scheduleScanCycleStop` are expected to run on the main thread. See Also: - https://developer.android.com/reference/android/os/Handler.html#Handler%28%29 - https://developer.android.com/training/multiple-threads/communicate-ui.html - https://developer.android.com/reference/android/app/Service.html - https://developer.android.com/guide/components/services.html - #430
1 parent d2f1f9f commit 39a98f4

File tree

2 files changed

+23
-4
lines changed

2 files changed

+23
-4
lines changed

src/main/java/org/altbeacon/beacon/service/BeaconService.java

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,8 +37,10 @@
3737
import android.os.Build;
3838
import android.os.Handler;
3939
import android.os.IBinder;
40+
import android.os.Looper;
4041
import android.os.Message;
4142
import android.os.Messenger;
43+
import android.support.annotation.MainThread;
4244

4345
import org.altbeacon.beacon.Beacon;
4446
import org.altbeacon.beacon.BeaconManager;
@@ -144,9 +146,16 @@ static class IncomingHandler extends Handler {
144146
private final WeakReference<BeaconService> mService;
145147

146148
IncomingHandler(BeaconService service) {
149+
/*
150+
* Explicitly state this uses the main thread. Without this we defer to where the
151+
* service instance is initialized/created; which is usually the main thread anyways.
152+
* But by being explicit we document our code design expectations for where things run.
153+
*/
154+
super(Looper.getMainLooper());
147155
mService = new WeakReference<BeaconService>(service);
148156
}
149157

158+
@MainThread
150159
@Override
151160
public void handleMessage(Message msg) {
152161
BeaconService service = mService.get();
@@ -205,7 +214,7 @@ else if (msg.what == MSG_SYNC_SETTINGS) {
205214
*/
206215
final Messenger mMessenger = new Messenger(new IncomingHandler(this));
207216

208-
217+
@MainThread
209218
@Override
210219
public void onCreate() {
211220
bluetoothCrashResolver = new BluetoothCrashResolver(this);
@@ -293,6 +302,7 @@ public boolean onUnbind(Intent intent) {
293302
return false;
294303
}
295304

305+
@MainThread
296306
@Override
297307
public void onDestroy() {
298308
LogManager.e(TAG, "onDestroy()");
@@ -329,6 +339,7 @@ private PendingIntent getRestartIntent() {
329339
/**
330340
* methods for clients
331341
*/
342+
@MainThread
332343
public void startRangingBeaconsInRegion(Region region, Callback callback) {
333344
synchronized (rangedRegionState) {
334345
if (rangedRegionState.containsKey(region)) {
@@ -341,6 +352,7 @@ public void startRangingBeaconsInRegion(Region region, Callback callback) {
341352
mCycledScanner.start();
342353
}
343354

355+
@MainThread
344356
public void stopRangingBeaconsInRegion(Region region) {
345357
int rangedRegionCount;
346358
synchronized (rangedRegionState) {
@@ -354,13 +366,15 @@ public void stopRangingBeaconsInRegion(Region region) {
354366
}
355367
}
356368

369+
@MainThread
357370
public void startMonitoringBeaconsInRegion(Region region, Callback callback) {
358371
LogManager.d(TAG, "startMonitoring called");
359372
monitoringStatus.addRegion(region, callback);
360373
LogManager.d(TAG, "Currently monitoring %s regions.", monitoringStatus.regionsCount());
361374
mCycledScanner.start();
362375
}
363376

377+
@MainThread
364378
public void stopMonitoringBeaconsInRegion(Region region) {
365379
LogManager.d(TAG, "stopMonitoring called");
366380
monitoringStatus.removeRegion(region);
@@ -370,6 +384,7 @@ public void stopMonitoringBeaconsInRegion(Region region) {
370384
}
371385
}
372386

387+
@MainThread
373388
public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean backgroundFlag) {
374389
mCycledScanner.setScanPeriods(scanPeriod, betweenScanPeriod, backgroundFlag);
375390
}

src/main/java/org/altbeacon/beacon/service/scanner/CycledLeScanner.java

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
package org.altbeacon.beacon.service.scanner;
22

33
import android.Manifest;
4-
import android.annotation.SuppressLint;
54
import android.annotation.TargetApi;
65
import android.app.AlarmManager;
76
import android.app.PendingIntent;
@@ -125,6 +124,7 @@ public static CycledLeScanner createScanner(Context context, long scanPeriod, lo
125124
* between LOW_POWER_MODE vs. LOW_LATENCY_MODE
126125
* @param backgroundFlag
127126
*/
127+
@MainThread
128128
public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean backgroundFlag) {
129129
LogManager.d(TAG, "Set scan periods called with %s, %s Background mode must have changed.",
130130
scanPeriod, betweenScanPeriod);
@@ -165,6 +165,7 @@ public void setScanPeriods(long scanPeriod, long betweenScanPeriod, boolean back
165165
}
166166
}
167167

168+
@MainThread
168169
public void start() {
169170
LogManager.d(TAG, "start called");
170171
mScanningEnabled = true;
@@ -175,7 +176,7 @@ public void start() {
175176
}
176177
}
177178

178-
@SuppressLint("NewApi")
179+
@MainThread
179180
public void stop() {
180181
LogManager.d(TAG, "stop called");
181182
mScanningEnabled = false;
@@ -194,6 +195,7 @@ public void setDistinctPacketsDetectedPerScan(boolean detected) {
194195
mDistinctPacketsDetectedPerScan = detected;
195196
}
196197

198+
@MainThread
197199
public void destroy() {
198200
LogManager.d(TAG, "Destroying");
199201
// We cannot quit the thread used by the handler until queued Runnables have been processed,
@@ -218,7 +220,7 @@ public void run() {
218220

219221
protected abstract void startScan();
220222

221-
@SuppressLint("NewApi")
223+
@MainThread
222224
protected void scanLeDevice(final Boolean enable) {
223225
try {
224226
mScanCyclerStarted = true;
@@ -285,6 +287,7 @@ protected void scanLeDevice(final Boolean enable) {
285287
}
286288
}
287289

290+
@MainThread
288291
protected void scheduleScanCycleStop() {
289292
// Stops scanning after a pre-defined scan period.
290293
long millisecondsUntilStop = mScanCycleStopTime - SystemClock.elapsedRealtime();
@@ -308,6 +311,7 @@ public void run() {
308311

309312
protected abstract void finishScan();
310313

314+
@MainThread
311315
private void finishScanCycle() {
312316
LogManager.d(TAG, "Done with scan cycle");
313317
try {

0 commit comments

Comments
 (0)