Skip to content

Commit 5c24fd0

Browse files
author
Victoria Lease
committed
Avoid NPE in GpsLocationProvider
Oops, looks like we were spinning up a secondary thread to run some tasks that will just happen on the main thread regardless. Removed the secondary thread and fixed up initialisation order regarding mHandler and things that post to it. Also reordered GPS and PASSIVE provider initialisation order since GPS depends on PASSIVE. This should be both safer and easier to read. Bug: 7248029 Change-Id: I8630caf0a7bd1b2c401603075676f13dda5be4fa
1 parent ce803d8 commit 5c24fd0

2 files changed

Lines changed: 37 additions & 65 deletions

File tree

services/java/com/android/server/LocationManagerService.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -228,6 +228,12 @@ public void onChange(boolean selfChange) {
228228
}
229229

230230
private void loadProvidersLocked() {
231+
// create a passive location provider, which is always enabled
232+
PassiveProvider passiveProvider = new PassiveProvider(this);
233+
addProviderLocked(passiveProvider);
234+
mEnabledProviders.add(passiveProvider.getName());
235+
mPassiveProvider = passiveProvider;
236+
231237
if (GpsLocationProvider.isSupported()) {
232238
// Create a gps location provider
233239
GpsLocationProvider gpsProvider = new GpsLocationProvider(mContext, this);
@@ -237,12 +243,6 @@ private void loadProvidersLocked() {
237243
mRealProviders.put(LocationManager.GPS_PROVIDER, gpsProvider);
238244
}
239245

240-
// create a passive location provider, which is always enabled
241-
PassiveProvider passiveProvider = new PassiveProvider(this);
242-
addProviderLocked(passiveProvider);
243-
mEnabledProviders.add(passiveProvider.getName());
244-
mPassiveProvider = passiveProvider;
245-
246246
/*
247247
Load package name(s) containing location provider support.
248248
These packages can contain services implementing location providers:

services/java/com/android/server/location/GpsLocationProvider.java

Lines changed: 31 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,8 @@
4040
import android.os.Bundle;
4141
import android.os.Handler;
4242
import android.os.IBinder;
43-
import android.os.Looper;
4443
import android.os.Message;
4544
import android.os.PowerManager;
46-
import android.os.Process;
4745
import android.os.RemoteException;
4846
import android.os.ServiceManager;
4947
import android.os.SystemClock;
@@ -75,7 +73,6 @@
7573
import java.util.Date;
7674
import java.util.Map.Entry;
7775
import java.util.Properties;
78-
import java.util.concurrent.CountDownLatch;
7976

8077
/**
8178
* A GPS implementation of LocationProvider used by LocationManager.
@@ -287,12 +284,8 @@ public GpsRequest(ProviderRequest request, WorkSource source) {
287284
private Bundle mLocationExtras = new Bundle();
288285
private ArrayList<Listener> mListeners = new ArrayList<Listener>();
289286

290-
// GpsLocationProvider's handler thread
291-
private final Thread mThread;
292-
// Handler for processing events in mThread.
287+
// Handler for processing events
293288
private Handler mHandler;
294-
// Used to signal when our main thread has initialized everything
295-
private final CountDownLatch mInitializedLatch = new CountDownLatch(1);
296289

297290
private String mAGpsApn;
298291
private int mAGpsDataConnectionState;
@@ -437,21 +430,6 @@ public GpsLocationProvider(Context context, ILocationManager ilocationManager) {
437430
mWakeupIntent = PendingIntent.getBroadcast(mContext, 0, new Intent(ALARM_WAKEUP), 0);
438431
mTimeoutIntent = PendingIntent.getBroadcast(mContext, 0, new Intent(ALARM_TIMEOUT), 0);
439432

440-
IntentFilter intentFilter = new IntentFilter();
441-
intentFilter.addAction(Intents.DATA_SMS_RECEIVED_ACTION);
442-
intentFilter.addDataScheme("sms");
443-
intentFilter.addDataAuthority("localhost","7275");
444-
context.registerReceiver(mBroadcastReciever, intentFilter);
445-
446-
intentFilter = new IntentFilter();
447-
intentFilter.addAction(Intents.WAP_PUSH_RECEIVED_ACTION);
448-
try {
449-
intentFilter.addDataType("application/vnd.omaloc-supl-init");
450-
} catch (IntentFilter.MalformedMimeTypeException e) {
451-
Log.w(TAG, "Malformed SUPL init mime type");
452-
}
453-
context.registerReceiver(mBroadcastReciever, intentFilter);
454-
455433
mConnMgr = (ConnectivityManager)context.getSystemService(Context.CONNECTIVITY_SERVICE);
456434

457435
// Battery statistics service to be notified when GPS turns on or off
@@ -487,26 +465,43 @@ public GpsLocationProvider(Context context, ILocationManager ilocationManager) {
487465
Log.w(TAG, "Could not open GPS configuration file " + PROPERTIES_FILE);
488466
}
489467

490-
// wait until we are fully initialized before returning
491-
mThread = new GpsLocationProviderThread();
492-
mThread.start();
493-
while (true) {
494-
try {
495-
mInitializedLatch.await();
496-
break;
497-
} catch (InterruptedException e) {
498-
Thread.currentThread().interrupt();
468+
// construct handler, listen for events
469+
mHandler = new ProviderHandler();
470+
listenForBroadcasts();
471+
472+
// also listen for PASSIVE_PROVIDER updates
473+
mHandler.post(new Runnable() {
474+
@Override
475+
public void run() {
476+
LocationManager locManager =
477+
(LocationManager) mContext.getSystemService(Context.LOCATION_SERVICE);
478+
locManager.requestLocationUpdates(LocationManager.PASSIVE_PROVIDER,
479+
0, 0, new NetworkLocationListener(), mHandler.getLooper());
499480
}
500-
}
481+
});
501482
}
502483

503-
private void initialize() {
504-
// register our receiver on our thread rather than the main thread
484+
private void listenForBroadcasts() {
505485
IntentFilter intentFilter = new IntentFilter();
486+
intentFilter.addAction(Intents.DATA_SMS_RECEIVED_ACTION);
487+
intentFilter.addDataScheme("sms");
488+
intentFilter.addDataAuthority("localhost","7275");
489+
mContext.registerReceiver(mBroadcastReciever, intentFilter, null, mHandler);
490+
491+
intentFilter = new IntentFilter();
492+
intentFilter.addAction(Intents.WAP_PUSH_RECEIVED_ACTION);
493+
try {
494+
intentFilter.addDataType("application/vnd.omaloc-supl-init");
495+
} catch (IntentFilter.MalformedMimeTypeException e) {
496+
Log.w(TAG, "Malformed SUPL init mime type");
497+
}
498+
mContext.registerReceiver(mBroadcastReciever, intentFilter, null, mHandler);
499+
500+
intentFilter = new IntentFilter();
506501
intentFilter.addAction(ALARM_WAKEUP);
507502
intentFilter.addAction(ALARM_TIMEOUT);
508503
intentFilter.addAction(ConnectivityManager.CONNECTIVITY_ACTION);
509-
mContext.registerReceiver(mBroadcastReciever, intentFilter);
504+
mContext.registerReceiver(mBroadcastReciever, intentFilter, null, mHandler);
510505
}
511506

512507
/**
@@ -1536,29 +1531,6 @@ public void handleMessage(Message msg) {
15361531
}
15371532
};
15381533

1539-
private final class GpsLocationProviderThread extends Thread {
1540-
1541-
public GpsLocationProviderThread() {
1542-
super("GpsLocationProvider");
1543-
}
1544-
1545-
@Override
1546-
public void run() {
1547-
Process.setThreadPriority(Process.THREAD_PRIORITY_BACKGROUND);
1548-
initialize();
1549-
Looper.prepare();
1550-
1551-
LocationManager locManager =
1552-
(LocationManager) mContext.getSystemService(Context.LOCATION_SERVICE);
1553-
mHandler = new ProviderHandler();
1554-
// signal when we are initialized and ready to go
1555-
mInitializedLatch.countDown();
1556-
locManager.requestLocationUpdates(LocationManager.PASSIVE_PROVIDER,
1557-
0, 0, new NetworkLocationListener(), Looper.myLooper());
1558-
Looper.loop();
1559-
}
1560-
}
1561-
15621534
private final class NetworkLocationListener implements LocationListener {
15631535
@Override
15641536
public void onLocationChanged(Location location) {

0 commit comments

Comments
 (0)