Fix potential deadlock in ServiceWatcher (192710b5) · Commits · e / os / android_frameworks_base

services/core/java/com/android/server/ServiceWatcher.java

+97 −55

Original line number	Diff line number	Diff line
		@@ -34,11 +34,11 @@ import android.os.Bundle;
		import android.os.Handler;
		import android.os.IBinder;
		import android.os.Looper;
		import android.os.RemoteException;
		import android.os.UserHandle;
		import android.util.Log;
		import android.util.Slog;

		import com.android.internal.annotations.GuardedBy;
		import com.android.internal.content.PackageMonitor;
		import com.android.internal.util.Preconditions;

		@@ -48,6 +48,9 @@ import java.util.Collections;
		import java.util.HashSet;
		import java.util.List;
		import java.util.Objects;
		import java.util.concurrent.Callable;
		import java.util.concurrent.ExecutionException;
		import java.util.concurrent.FutureTask;

		/**
		* Find the best Service, and bind to it.
		@@ -61,16 +64,20 @@ public class ServiceWatcher implements ServiceConnection {
		public static final String EXTRA_SERVICE_VERSION = "serviceVersion";
		public static final String EXTRA_SERVICE_IS_MULTIUSER = "serviceIsMultiuser";

		/**
		* The runner that runs on the binder retrieved from {@link ServiceWatcher}.
		*/

		/** Function to run on binder interface. */
		public interface BinderRunner {
		/** Called to run client code with the binder. */
		void run(IBinder binder) throws RemoteException;
		}

		/**
		* Runs on the retrieved binder.
		*
		* @param binder the binder retrieved from the {@link ServiceWatcher}.
		* Function to run on binder interface.
		* @param <T> Type to return.
		*/
		void run(IBinder binder);
		public interface BlockingBinderRunner<T> {
		/** Called to run client code with the binder. */
		T run(IBinder binder) throws RemoteException;
		}

		public static ArrayList<HashSet<Signature>> getSignatureSets(Context context,
		@@ -120,18 +127,14 @@ public class ServiceWatcher implements ServiceConnection {

		private final Handler mHandler;

		// this lock is held to ensure the service binder is not exposed (via runOnBinder) until after
		// the new service initialization work has completed
		private final Object mBindLock = new Object();

		// read/write from handler thread
		private IBinder mBestService;
		private int mCurrentUserId;

		// read from any thread, write from handler thread
		private volatile ComponentName mBestComponent;
		private volatile int mBestVersion;
		private volatile int mBestUserId;
		private volatile IBinder mBestService;

		public ServiceWatcher(Context context, String logTag, String action,
		int overlaySwitchResId, int defaultServicePackageNameResId,
		@@ -163,17 +166,9 @@ public class ServiceWatcher implements ServiceConnection {
		mBestService = null;
		}

		// called on handler thread
		@GuardedBy("mBindLock")
		protected void onBind() {
		Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());
		}
		protected void onBind() {}

		// called on handler thread
		@GuardedBy("mBindLock")
		protected void onUnbind() {
		Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());
		}
		protected void onUnbind() {}

		/**
		* Start this watcher, including binding to the current best match and
		@@ -248,25 +243,6 @@ public class ServiceWatcher implements ServiceConnection {
		return bestComponent == null ? null : bestComponent.getPackageName();
		}

		/**
		* Runs the given BinderRunner if currently connected. All invocations to runOnBinder are run
		* serially.
		*/
		public final void runOnBinder(BinderRunner runner) {
		synchronized (mBindLock) {
		IBinder service = mBestService;
		if (service != null) {
		try {
		runner.run(service);
		} catch (Exception e) {
		// remote exceptions cannot be allowed to crash system server
		Log.e(TAG, "exception while while running " + runner + " on " + service
		+ " from " + this, e);
		}
		}
		}
		}

		private boolean isServiceMissing() {
		return mContext.getPackageManager().queryIntentServicesAsUser(new Intent(mAction),
		PackageManager.MATCH_DIRECT_BOOT_AWARE
		@@ -380,28 +356,66 @@ public class ServiceWatcher implements ServiceConnection {
		mBestUserId = UserHandle.USER_NULL;
		}

		/**
		* Runs the given function asynchronously if currently connected. Suppresses any RemoteException
		* thrown during execution.
		*/
		public final void runOnBinder(BinderRunner runner) {
		runOnHandler(() -> {
		if (mBestService == null) {
		return;
		}

		try {
		runner.run(mBestService);
		} catch (RuntimeException e) {
		// the code being run is privileged, but may be outside the system server, and thus
		// we cannot allow runtime exceptions to crash the system server
		Log.e(TAG, "exception while while running " + runner + " on " + mBestService
		+ " from " + this, e);
		} catch (RemoteException e) {
		// do nothing
		}
		});
		}

		/**
		* Runs the given function synchronously if currently connected, and returns the default value
		* if not currently connected or if any exception is thrown.
		*/
		public final <T> T runOnBinderBlocking(BlockingBinderRunner<T> runner, T defaultValue) {
		try {
		return runOnHandlerBlocking(() -> {
		if (mBestService == null) {
		return defaultValue;
		}

		try {
		return runner.run(mBestService);
		} catch (RemoteException e) {
		return defaultValue;
		}
		});
		} catch (InterruptedException e) {
		return defaultValue;
		}
		}

		@Override
		public final void onServiceConnected(ComponentName component, IBinder binder) {
		mHandler.post(() -> {
		runOnHandler(() -> {
		if (D) Log.d(mTag, component + " connected");

		// hold the lock so that mBestService cannot be used by runOnBinder until complete
		synchronized (mBindLock) {
		mBestService = binder;
		onBind();
		}
		});
		}

		@Override
		public final void onServiceDisconnected(ComponentName component) {
		mHandler.post(() -> {
		runOnHandler(() -> {
		if (D) Log.d(mTag, component + " disconnected");

		mBestService = null;
		synchronized (mBindLock) {
		onUnbind();
		}
		});
		}

		@@ -410,4 +424,32 @@ public class ServiceWatcher implements ServiceConnection {
		ComponentName bestComponent = mBestComponent;
		return bestComponent == null ? "null" : bestComponent.toShortString() + "@" + mBestVersion;
		}

		private void runOnHandler(Runnable r) {
		if (Looper.myLooper() == mHandler.getLooper()) {
		r.run();
		} else {
		mHandler.post(r);
		}
		}

		private <T> T runOnHandlerBlocking(Callable<T> c) throws InterruptedException {
		if (Looper.myLooper() == mHandler.getLooper()) {
		try {
		return c.call();
		} catch (Exception e) {
		// Function cannot throw exception, this should never happen
		throw new IllegalStateException(e);
		}
		} else {
		FutureTask<T> task = new FutureTask<>(c);
		mHandler.post(task);
		try {
		return task.get();
		} catch (ExecutionException e) {
		// Function cannot throw exception, this should never happen
		throw new IllegalStateException(e);
		}
		}
		}
		}

services/core/java/com/android/server/location/GeocoderProxy.java

+8 −23

Original line number	Diff line number	Diff line
		@@ -20,8 +20,6 @@ import android.content.Context;
		import android.location.Address;
		import android.location.GeocoderParams;
		import android.location.IGeocodeProvider;
		import android.os.RemoteException;
		import android.util.Log;

		import com.android.internal.os.BackgroundThread;
		import com.android.server.ServiceWatcher;
		@@ -68,35 +66,22 @@ public class GeocoderProxy {

		public String getFromLocation(double latitude, double longitude, int maxResults,
		GeocoderParams params, List<Address> addrs) {
		final String[] result = new String[]{"Service not Available"};
		mServiceWatcher.runOnBinder(binder -> {
		return mServiceWatcher.runOnBinderBlocking(binder -> {
		IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
		try {
		result[0] = provider.getFromLocation(
		latitude, longitude, maxResults, params, addrs);
		} catch (RemoteException e) {
		Log.w(TAG, e);
		}
		});
		return result[0];
		return provider.getFromLocation(latitude, longitude, maxResults, params, addrs);
		}, "Service not Available");
		}

		public String getFromLocationName(String locationName,
		double lowerLeftLatitude, double lowerLeftLongitude,
		double upperRightLatitude, double upperRightLongitude, int maxResults,
		GeocoderParams params, List<Address> addrs) {
		final String[] result = new String[]{"Service not Available"};
		mServiceWatcher.runOnBinder(binder -> {
		return mServiceWatcher.runOnBinderBlocking(binder -> {
		IGeocodeProvider provider = IGeocodeProvider.Stub.asInterface(binder);
		try {
		result[0] = provider.getFromLocationName(locationName, lowerLeftLatitude,
		return provider.getFromLocationName(locationName, lowerLeftLatitude,
		lowerLeftLongitude, upperRightLatitude, upperRightLongitude,
		maxResults, params, addrs);
		} catch (RemoteException e) {
		Log.w(TAG, e);
		}
		});
		return result[0];
		}, "Service not Available");
		}

		}

services/core/java/com/android/server/location/LocationProviderProxy.java

+17 −40

Original line number	Diff line number	Diff line
		@@ -127,11 +127,10 @@ public class LocationProviderProxy extends AbstractLocationProvider {
		return mServiceWatcher.start();
		}

		private void initializeService(IBinder binder) {
		private void initializeService(IBinder binder) throws RemoteException {
		ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
		if (D) Log.d(TAG, "applying state to connected service " + mServiceWatcher);

		try {
		service.setLocationProviderManager(mManager);

		synchronized (mRequestLock) {
		@@ -139,9 +138,6 @@ public class LocationProviderProxy extends AbstractLocationProvider {
		service.setRequest(mRequest, mWorkSource);
		}
		}
		} catch (RemoteException e) {
		Log.w(TAG, e);
		}
		}

		@Nullable
		@@ -157,63 +153,44 @@ public class LocationProviderProxy extends AbstractLocationProvider {
		}
		mServiceWatcher.runOnBinder(binder -> {
		ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
		try {
		service.setRequest(request, source);
		} catch (RemoteException e) {
		Log.w(TAG, e);
		}
		});
		}

		@Override
		public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
		pw.println(" service=" + mServiceWatcher);
		mServiceWatcher.runOnBinder(binder -> {
		mServiceWatcher.runOnBinderBlocking(binder -> {
		try {
		TransferPipe.dumpAsync(binder, fd, args);
		} catch (IOException \| RemoteException e) {
		pw.println(" failed to dump location provider: " + e);
		pw.println(" failed to dump location provider");
		}
		});
		return null;
		}, null);
		}

		@Override
		public int getStatus(Bundle extras) {
		int[] status = new int[] {LocationProvider.TEMPORARILY_UNAVAILABLE};
		mServiceWatcher.runOnBinder(binder -> {
		return mServiceWatcher.runOnBinderBlocking(binder -> {
		ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
		try {
		status[0] = service.getStatus(extras);
		} catch (RemoteException e) {
		Log.w(TAG, e);
		}
		});
		return status[0];
		return service.getStatus(extras);
		}, LocationProvider.TEMPORARILY_UNAVAILABLE);
		}

		@Override
		public long getStatusUpdateTime() {
		long[] updateTime = new long[] {0L};
		mServiceWatcher.runOnBinder(binder -> {
		return mServiceWatcher.runOnBinderBlocking(binder -> {
		ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
		try {
		updateTime[0] = service.getStatusUpdateTime();
		} catch (RemoteException e) {
		Log.w(TAG, e);
		}
		});
		return updateTime[0];
		return service.getStatusUpdateTime();
		}, 0L);
		}

		@Override
		public void sendExtraCommand(String command, Bundle extras) {
		mServiceWatcher.runOnBinder(binder -> {
		ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
		try {
		service.sendExtraCommand(command, extras);
		} catch (RemoteException e) {
		Log.w(TAG, e);
		}
		});
		}
		}