Fix possible deadlock (3b4e8a60) · Commits · e / os / android_frameworks_base

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

+38 −22

Original line number	Diff line number	Diff line
		@@ -49,6 +49,8 @@ import android.util.Log;
		import com.android.internal.content.PackageMonitor;
		import com.android.internal.util.Preconditions;

		import java.io.FileDescriptor;
		import java.io.PrintWriter;
		import java.util.List;
		import java.util.Objects;
		import java.util.concurrent.Callable;
		@@ -183,9 +185,7 @@ public class ServiceWatcher implements ServiceConnection {

		// write from handler thread only, read anywhere
		private volatile ServiceInfo mServiceInfo;

		// read/write from handler thread only
		private IBinder mBinder;
		private volatile IBinder mBinder;

		public ServiceWatcher(Context context, Handler handler, String action,
		@Nullable BinderRunner onBind, @Nullable Runnable onUnbind,
		@@ -345,20 +345,25 @@ public class ServiceWatcher implements ServiceConnection {
		}

		mBinder = binder;

		// we always run the on bind callback even if we know that the binder is dead already so
		// that there are always balance pairs of bind/unbind callbacks
		if (mOnBind != null) {
		runOnBinder(mOnBind);
		try {
		mOnBind.run(binder);
		} catch (RuntimeException \| RemoteException e) {
		// binders may propagate some specific non-RemoteExceptions from the other side
		// through the binder as well - we cannot allow those to crash the system server
		Log.e(TAG, getLogPrefix() + " exception running on " + mServiceInfo, e);
		}
		}

		@Override
		public void onBindingDied(ComponentName component) {
		Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());

		if (D) {
		Log.i(TAG, getLogPrefix() + " " + component.toShortString() + " died");
		try {
		// setting the binder to null lets us skip queued transactions
		binder.linkToDeath(() -> mBinder = null, 0);
		} catch (RemoteException e) {
		mBinder = null;
		}

		onBestServiceChanged(true);
		}

		@Override
		@@ -375,6 +380,17 @@ public class ServiceWatcher implements ServiceConnection {
		}
		}

		@Override
		public void onBindingDied(ComponentName component) {
		Preconditions.checkState(Looper.myLooper() == mHandler.getLooper());

		if (D) {
		Log.i(TAG, getLogPrefix() + " " + component.toShortString() + " died");
		}

		onBestServiceChanged(true);
		}

		private void onUserSwitched(@UserIdInt int userId) {
		mCurrentUserId = userId;
		onBestServiceChanged(false);
		@@ -398,7 +414,7 @@ public class ServiceWatcher implements ServiceConnection {
		* RemoteException thrown during execution.
		*/
		public final void runOnBinder(BinderRunner runner) {
		runOnHandler(() -> {
		mHandler.post(() -> {
		if (mBinder == null) {
		return;
		}
		@@ -447,14 +463,6 @@ public class ServiceWatcher implements ServiceConnection {
		}
		}

		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, TimeoutException {
		if (Looper.myLooper() == mHandler.getLooper()) {
		@@ -489,4 +497,12 @@ public class ServiceWatcher implements ServiceConnection {
		public String toString() {
		return mServiceInfo.toString();
		}

		/**
		* Dump for debugging.
		*/
		public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
		pw.println("service=" + mServiceInfo);
		pw.println("connected=" + (mBinder != null));
		}
		}

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

+10 −14

Original line number	Diff line number	Diff line
		@@ -139,13 +139,13 @@ public class LocationProviderProxy extends AbstractLocationProvider {

		@GuardedBy("mLock")
		private boolean mBound;
		@GuardedBy("mLock")
		private ProviderRequest mRequest;

		private volatile ProviderRequest mRequest;

		private LocationProviderProxy(Context context, String action, int enableOverlayResId,
		int nonOverlayPackageResId) {
		// safe to use direct executor even though this class has internal locks - all of our
		// callbacks go to oneway binder transactions which cannot possibly be re-entrant
		// safe to use direct executor since our locks are not acquired in a code path invoked by
		// our owning provider
		super(DIRECT_EXECUTOR, Collections.emptySet());

		mContext = context;
		@@ -167,8 +167,10 @@ public class LocationProviderProxy extends AbstractLocationProvider {
		mBound = true;

		provider.setLocationProviderManager(mManager);
		if (!mRequest.equals(ProviderRequest.EMPTY_REQUEST)) {
		provider.setRequest(mRequest, mRequest.workSource);

		ProviderRequest request = mRequest;
		if (!request.equals(ProviderRequest.EMPTY_REQUEST)) {
		provider.setRequest(request, request.workSource);
		}

		ComponentName service = mServiceWatcher.getBoundService().component;
		@@ -187,10 +189,7 @@ public class LocationProviderProxy extends AbstractLocationProvider {

		@Override
		public void onSetRequest(ProviderRequest request) {
		synchronized (mLock) {
		mRequest = request;
		}

		mServiceWatcher.runOnBinder(binder -> {
		ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
		service.setRequest(request, request.workSource);
		@@ -215,9 +214,6 @@ public class LocationProviderProxy extends AbstractLocationProvider {

		@Override
		public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
		pw.println("service=" + mServiceWatcher);
		synchronized (mLock) {
		pw.println("bound=" + mBound);
		}
		mServiceWatcher.dump(fd, pw, args);
		}
		}