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

Commit d334f7b5 authored by Soonil Nagarkar's avatar Soonil Nagarkar
Browse files

Make test APIs synchronous

Post outgoing commands to providers to a handler rather than posting
incoming requests from providers to a handler in order to break deadlock
loops within providers. This makes it easier to the test APIs to behave
synchronously as a client would expect.

Bug: 136638281
Test: CTS
Change-Id: Id8ec2c18f287a31148cb854019ee52012db2a806
parent 1aaa574c
Loading
Loading
Loading
Loading
+56 −69
Original line number Diff line number Diff line
@@ -905,7 +905,8 @@ public class LocationManagerService extends ILocationManager.Stub {
                    Integer.parseInt(fragments[9]) /* accuracy */);
            LocationProvider testProviderManager = new LocationProvider(name);
            addProviderLocked(testProviderManager);
            new MockProvider(mContext, testProviderManager, properties);
            testProviderManager.attachLocked(
                    new MockProvider(mContext, testProviderManager, properties));
        }
    }

@@ -1026,16 +1027,29 @@ public class LocationManagerService extends ILocationManager.Stub {
            return mProperties;
        }

        @GuardedBy("mLock")
        public void setRequestLocked(ProviderRequest request, WorkSource workSource) {
        public void setRequest(ProviderRequest request, WorkSource workSource) {
            // move calls going to providers onto a different thread to avoid deadlock
            mHandler.post(() -> {
                synchronized (mLock) {
                    if (mProvider != null) {
                long identity = Binder.clearCallingIdentity();
                try {
                    mProvider.setRequest(request, workSource);
                } finally {
                    Binder.restoreCallingIdentity(identity);
                        mProvider.onSetRequest(request, workSource);
                    }
                }
            });
        }

        public void sendExtraCommand(String command, Bundle extras) {
            int uid = Binder.getCallingUid();
            int pid = Binder.getCallingPid();

            // move calls going to providers onto a different thread to avoid deadlock
            mHandler.post(() -> {
                synchronized (mLock) {
                    if (mProvider != null) {
                        mProvider.onSendExtraCommand(uid, pid, command, extras);
                    }
                }
            });
        }

        @GuardedBy("mLock")
@@ -1095,32 +1109,15 @@ public class LocationManagerService extends ILocationManager.Stub {
            }
        }

        @GuardedBy("mLock")
        public void sendExtraCommandLocked(String command, Bundle extras) {
            if (mProvider != null) {
                // intentionally do not clear binder identity so that providers can evaluate who
                // is sending the extra command
                mProvider.sendExtraCommand(command, extras);
            }
        }

        // called from any thread
        @Override
        public void onReportLocation(Location location) {
            // no security check necessary because this is coming from an internal-only interface
            // move calls coming from below LMS onto a different thread to avoid deadlock
            mHandler.post(() -> {
            synchronized (mLock) {
                handleLocationChangedLocked(location, this);
            }
            });
        }

        // called from any thread
        @Override
        public void onReportLocation(List<Location> locations) {
            // move calls coming from below LMS onto a different thread to avoid deadlock
            mHandler.post(() -> {
            synchronized (mLock) {
                LocationProvider gpsProvider = getLocationProviderLocked(GPS_PROVIDER);
                if (gpsProvider == null || !gpsProvider.isUseableLocked()) {
@@ -1139,14 +1136,10 @@ public class LocationManagerService extends ILocationManager.Stub {
                    Slog.e(TAG, "mGnssBatchingCallback.onLocationBatch failed", e);
                }
            }
            });
        }

        // called from any thread
        @Override
        public void onSetEnabled(boolean enabled) {
            // move calls coming from below LMS onto a different thread to avoid deadlock
            mHandler.post(() -> {
            synchronized (mLock) {
                if (enabled == mEnabled) {
                    return;
@@ -1159,15 +1152,10 @@ public class LocationManagerService extends ILocationManager.Stub {
                mEnabled = enabled;
                onUseableChangedLocked(false);
            }
            });
        }

        @Override
        public void onSetProperties(ProviderProperties properties) {
            // because this does not invoke any other methods which might result in calling back
            // into the location provider, it is safe to run this on the calling thread. it is also
            // currently necessary to run this on the calling thread to ensure that property changes
            // are publicly visibly immediately, ie for mock providers which are created.
            synchronized (mLock) {
                mProperties = properties;
            }
@@ -1325,9 +1313,8 @@ public class LocationManagerService extends ILocationManager.Stub {
        }

        @Override
        @GuardedBy("mLock")
        public void setRequestLocked(ProviderRequest request, WorkSource workSource) {
            super.setRequestLocked(request, workSource);
        public void setRequest(ProviderRequest request, WorkSource workSource) {
            super.setRequest(request, workSource);
            mCurrentRequest = request;
        }

@@ -2232,7 +2219,7 @@ public class LocationManagerService extends ILocationManager.Stub {
            }
        }

        provider.setRequestLocked(providerRequest, worksource);
        provider.setRequest(providerRequest, worksource);
    }

    /**
@@ -3116,7 +3103,7 @@ public class LocationManagerService extends ILocationManager.Stub {

            LocationProvider provider = getLocationProviderLocked(providerName);
            if (provider != null) {
                provider.sendExtraCommandLocked(command, extras);
                provider.sendExtraCommand(command, extras);
            }

            mLocationUsageLogger.logLocationApiUsage(
+33 −26
Original line number Diff line number Diff line
@@ -76,6 +76,22 @@ public abstract class AbstractLocationProvider {
        mLocationProviderManager = locationProviderManager;
    }

    /**
     * Call this method to report a change in provider enabled/disabled status. May be called from
     * any thread.
     */
    protected void setEnabled(boolean enabled) {
        mLocationProviderManager.onSetEnabled(enabled);
    }

    /**
     * Call this method to report a change in provider properties. May be called from
     * any thread.
     */
    protected void setProperties(ProviderProperties properties) {
        mLocationProviderManager.onSetProperties(properties);
    }

    /**
     * Call this method to report a new location. May be called from any thread.
     */
@@ -91,39 +107,35 @@ public abstract class AbstractLocationProvider {
    }

    /**
     * Call this method to report a change in provider enabled/disabled status. May be called from
     * any thread.
     * Invoked by the location service to return a list of packages currently associated with this
     * provider. May be called from any thread.
     */
    protected void setEnabled(boolean enabled) {
        mLocationProviderManager.onSetEnabled(enabled);
    public List<String> getProviderPackages() {
        return Collections.singletonList(mContext.getPackageName());
    }

    /**
     * Call this method to report a change in provider properties. May be called from
     * any thread.
     * Invoked by the location service to deliver a new request for fulfillment to the provider.
     * Replaces any previous requests completely. Will always be invoked from the location service
     * thread with a cleared binder identity.
     */
    protected void setProperties(ProviderProperties properties) {
        mLocationProviderManager.onSetProperties(properties);
    }

    /** Returns list of packages currently associated with this provider. */
    public List<String> getProviderPackages() {
        return Collections.singletonList(mContext.getPackageName());
    }
    public abstract void onSetRequest(ProviderRequest request, WorkSource source);

    /**
     * Called when the location service delivers a new request for fulfillment to the provider.
     * Replaces any previous requests completely.
     * Invoked by the location service to deliver a custom command to this provider. Will always be
     * invoked from the location service thread with a cleared binder identity.
     */
    public abstract void setRequest(ProviderRequest request, WorkSource source);
    public void onSendExtraCommand(int uid, int pid, String command, Bundle extras) {}

    /**
     * Called to dump debug or log information.
     * Invoked by the location service to dump debug or log information. May be invoked from any
     * thread.
     */
    public abstract void dump(FileDescriptor fd, PrintWriter pw, String[] args);

    /**
     * Retrieves the current status of the provider.
     * Invoked by the location service to retrieve the current status of the provider. May be
     * invoked from any thread.
     *
     * @deprecated Will be removed in a future release.
     */
@@ -133,7 +145,8 @@ public abstract class AbstractLocationProvider {
    }

    /**
     * Retrieves the last update time of the status of the provider.
     * Invoked by the location service to retrieve the last update time of the status of the
     * provider. May be invoked from any thread.
     *
     * @deprecated Will be removed in a future release.
     */
@@ -141,10 +154,4 @@ public abstract class AbstractLocationProvider {
    public long getStatusUpdateTime() {
        return 0;
    }

    /**
     * Sends a custom command to this provider. Called with the original binder identity of the
     * caller.
     */
    public abstract void sendExtraCommand(String command, Bundle extras);
}
+2 −2
Original line number Diff line number Diff line
@@ -1028,7 +1028,7 @@ public class GnssLocationProvider extends AbstractLocationProvider implements
    }

    @Override
    public void setRequest(ProviderRequest request, WorkSource source) {
    public void onSetRequest(ProviderRequest request, WorkSource source) {
        sendMessage(SET_REQUEST, 0, new GpsRequest(request, source));
    }

@@ -1165,7 +1165,7 @@ public class GnssLocationProvider extends AbstractLocationProvider implements
    }

    @Override
    public void sendExtraCommand(String command, Bundle extras) {
    public void onSendExtraCommand(int uid, int pid, String command, Bundle extras) {

        long identity = Binder.clearCallingIdentity();
        try {
+2 −2
Original line number Diff line number Diff line
@@ -168,7 +168,7 @@ public class LocationProviderProxy extends AbstractLocationProvider {
    }

    @Override
    public void setRequest(ProviderRequest request, WorkSource source) {
    public void onSetRequest(ProviderRequest request, WorkSource source) {
        synchronized (mRequestLock) {
            mRequest = request;
            mWorkSource = source;
@@ -206,7 +206,7 @@ public class LocationProviderProxy extends AbstractLocationProvider {
    }

    @Override
    public void sendExtraCommand(String command, Bundle extras) {
    public void onSendExtraCommand(int uid, int pid, String command, Bundle extras) {
        mServiceWatcher.runOnBinder(binder -> {
            ILocationProvider service = ILocationProvider.Stub.asInterface(binder);
            service.sendExtraCommand(command, extras);
+1 −4
Original line number Diff line number Diff line
@@ -85,7 +85,7 @@ public class MockProvider extends AbstractLocationProvider {
    }

    @Override
    public void setRequest(ProviderRequest request, WorkSource source) {}
    public void onSetRequest(ProviderRequest request, WorkSource source) {}

    @Override
    public int getStatus(Bundle extras) {
@@ -101,7 +101,4 @@ public class MockProvider extends AbstractLocationProvider {
    public long getStatusUpdateTime() {
        return mStatusUpdateTime;
    }

    @Override
    public void sendExtraCommand(String command, Bundle extras) {}
}
Loading