Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3b69cbef authored by Soonil Nagarkar's avatar Soonil Nagarkar Committed by Android (Google) Code Review
Browse files

Merge "Fix potential deadlock in ServiceWatcher"

parents efb4132b 192710b5
Loading
Loading
Loading
Loading
+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);
            }
        }
    }
}
+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");
    }

}
+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);
            }
        });
    }
}