Revert "Revert "Reduce allocations on location delivery"" (6bcfb687) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/location/provider/LocationProviderManager.java

+52 −11

Original line number	Diff line number	Diff line
		@@ -177,7 +177,7 @@ public class LocationProviderManager extends
		protected interface LocationTransport {

		void deliverOnLocationChanged(LocationResult locationResult,
		@Nullable Runnable onCompleteCallback) throws Exception;
		@Nullable IRemoteCallback onCompleteCallback) throws Exception;
		void deliverOnFlushComplete(int requestCode) throws Exception;
		}

		@@ -197,9 +197,8 @@ public class LocationProviderManager extends

		@Override
		public void deliverOnLocationChanged(LocationResult locationResult,
		@Nullable Runnable onCompleteCallback) throws RemoteException {
		mListener.onLocationChanged(locationResult.asList(),
		SingleUseCallback.wrap(onCompleteCallback));
		@Nullable IRemoteCallback onCompleteCallback) throws RemoteException {
		mListener.onLocationChanged(locationResult.asList(), onCompleteCallback);
		}

		@Override
		@@ -227,7 +226,7 @@ public class LocationProviderManager extends

		@Override
		public void deliverOnLocationChanged(LocationResult locationResult,
		@Nullable Runnable onCompleteCallback)
		@Nullable IRemoteCallback onCompleteCallback)
		throws PendingIntent.CanceledException {
		BroadcastOptions options = BroadcastOptions.makeBasic();
		options.setDontSendToRestrictedApps(true);
		@@ -243,20 +242,34 @@ public class LocationProviderManager extends
		intent.putExtra(KEY_LOCATIONS, locationResult.asList().toArray(new Location[0]));
		}

		PendingIntent.OnFinished onFinished = null;

		// send() SHOULD only run the completion callback if it completes successfully. however,
		// b/199464864 (which could not be fixed in the S timeframe) means that it's possible
		// b/201299281 (which could not be fixed in the S timeframe) means that it's possible
		// for send() to throw an exception AND run the completion callback. if this happens, we
		// would over-release the wakelock... we take matters into our own hands to ensure that
		// the completion callback can only be run if send() completes successfully. this means
		// the completion callback may be run inline - but as we've never specified what thread
		// the callback is run on, this is fine.
		GatedCallback gatedCallback = new GatedCallback(onCompleteCallback);
		GatedCallback gatedCallback;
		if (onCompleteCallback != null) {
		gatedCallback = new GatedCallback(() -> {
		try {
		onCompleteCallback.sendResult(null);
		} catch (RemoteException e) {
		throw e.rethrowFromSystemServer();
		}
		});
		onFinished = (pI, i, rC, rD, rE) -> gatedCallback.run();
		} else {
		gatedCallback = new GatedCallback(null);
		}

		mPendingIntent.send(
		mContext,
		0,
		intent,
		(pI, i, rC, rD, rE) -> gatedCallback.run(),
		onFinished,
		null,
		null,
		options.toBundle());
		@@ -293,7 +306,7 @@ public class LocationProviderManager extends

		@Override
		public void deliverOnLocationChanged(@Nullable LocationResult locationResult,
		@Nullable Runnable onCompleteCallback)
		@Nullable IRemoteCallback onCompleteCallback)
		throws RemoteException {
		// ILocationCallback doesn't currently support completion callbacks
		Preconditions.checkState(onCompleteCallback == null);
		@@ -714,6 +727,13 @@ public class LocationProviderManager extends

		final PowerManager.WakeLock mWakeLock;

		// b/206340085 - if we allocate a new wakelock releaser object for every delivery we
		// increase the risk of resource starvation. if a client stops processing deliveries the
		// system server binder allocation pool will be starved as we continue to queue up
		// deliveries, each with a new allocation. in order to mitigate this, we use a single
		// releaser object per registration rather than per delivery.
		final ExternalWakeLockReleaser mWakeLockReleaser;

		private volatile ProviderTransport mProviderTransport;
		private int mNumLocationsDelivered = 0;
		private long mExpirationRealtimeMs = Long.MAX_VALUE;
		@@ -727,6 +747,7 @@ public class LocationProviderManager extends
		.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, WAKELOCK_TAG);
		mWakeLock.setReferenceCounted(true);
		mWakeLock.setWorkSource(request.getWorkSource());
		mWakeLockReleaser = new ExternalWakeLockReleaser(identity, mWakeLock);
		}

		@Override
		@@ -943,7 +964,7 @@ public class LocationProviderManager extends
		}

		listener.deliverOnLocationChanged(deliverLocationResult,
		mUseWakeLock ? mWakeLock::release : null);
		mUseWakeLock ? mWakeLockReleaser : null);
		EVENT_LOG.logProviderDeliveredLocations(mName, locationResult.size(),
		getIdentity());
		}
		@@ -2761,7 +2782,7 @@ public class LocationProviderManager extends
		@GuardedBy("this")
		private boolean mRun;

		GatedCallback(Runnable callback) {
		GatedCallback(@Nullable Runnable callback) {
		mCallback = callback;
		}

		@@ -2796,4 +2817,24 @@ public class LocationProviderManager extends
		}
		}
		}

		private static class ExternalWakeLockReleaser extends IRemoteCallback.Stub {

		private final CallerIdentity mIdentity;
		private final PowerManager.WakeLock mWakeLock;

		ExternalWakeLockReleaser(CallerIdentity identity, PowerManager.WakeLock wakeLock) {
		mIdentity = identity;
		mWakeLock = Objects.requireNonNull(wakeLock);
		}

		@Override
		public void sendResult(Bundle data) {
		try {
		mWakeLock.release();
		} catch (RuntimeException e) {
		Log.e(TAG, "wakelock over-released by " + mIdentity, e);
		}
		}
		}
		}