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

Commit c23e5505 authored by Fabián Kozynski's avatar Fabián Kozynski
Browse files

Fix binder death and disconnection.

After this fix:
* If we just get a `onServiceDisconnected`, we will only clear the
  wrapper but won't unbind. According to the docs, this may reconnect by
  the system (the binder is still alive).
* If the binder dies (either detected by ServiceConnection or
  DeathRecipient), we clear the wrapper and unbind. After that, if we
  still want to be bound (mBound.get() == true), we will schedule a
  rebind later. We will only rebind if by that time, we still want to be
  bound.

Also add component name to log lines.

Fixes: 302657931
Test: atest TileLifecycleManagerTest
Test: manual, crash the tileservice package and see the rebinds (unless
the shade is closed).
Test: manual, check that at any time there's only one instance of the
binding (adb shell dumpsys activity services).
Flag: NONE

Change-Id: I1fdd8aaa62d175d292a3ad38f86eaec3582dd27a
parent 330a4fc2
Loading
Loading
Loading
Loading
+91 −59
Original line number Diff line number Diff line
@@ -66,7 +66,8 @@ import java.util.concurrent.atomic.AtomicBoolean;
 */
public class TileLifecycleManager extends BroadcastReceiver implements
        IQSTileService, ServiceConnection, IBinder.DeathRecipient {
    public static final boolean DEBUG = false;

    private final boolean mDebug = Log.isLoggable(TAG, Log.DEBUG);

    private static final String TAG = "TileLifecycleManager";

@@ -101,7 +102,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    private Set<Integer> mQueuedMessages = new ArraySet<>();
    @Nullable
    private QSTileServiceWrapper mWrapper;
    private volatile QSTileServiceWrapper mWrapper;
    private boolean mListening;
    private IBinder mClickBinder;

@@ -132,7 +133,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
        mPackageManagerAdapter = packageManagerAdapter;
        mBroadcastDispatcher = broadcastDispatcher;
        mActivityManager = activityManager;
        if (DEBUG) Log.d(TAG, "Creating " + mIntent + " " + mUser);
        if (mDebug) Log.d(TAG, "Creating " + mIntent + " " + mUser);
    }

    /** Injectable factory for TileLifecycleManager. */
@@ -215,10 +216,11 @@ public class TileLifecycleManager extends BroadcastReceiver implements
            if (!checkComponentState()) {
                return;
            }
            if (DEBUG) Log.d(TAG, "Binding service " + mIntent + " " + mUser);
            if (mDebug) Log.d(TAG, "Binding service " + mIntent + " " + mUser);
            mBindTryCount++;
            try {
                mIsBound.set(bindServices());
                // Only try a new binding if we are not currently bound.
                mIsBound.compareAndSet(false, bindServices());
                if (!mIsBound.get()) {
                    mContext.unbindService(this);
                }
@@ -227,19 +229,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
                mIsBound.set(false);
            }
        } else {
            if (DEBUG) Log.d(TAG, "Unbinding service " + mIntent + " " + mUser);
            // Give it another chance next time it needs to be bound, out of kindness.
            mBindTryCount = 0;
            freeWrapper();
            if (mIsBound.get()) {
                try {
                    mContext.unbindService(this);
                } catch (Exception e) {
                    Log.e(TAG, "Failed to unbind service "
                            + mIntent.getComponent().flattenToShortString(), e);
                }
                mIsBound.set(false);
            }
            unbindService();
        }
    }

@@ -264,9 +254,26 @@ public class TileLifecycleManager extends BroadcastReceiver implements
                mUser);
    }

    @WorkerThread
    private void unbindService() {
        if (mDebug) Log.d(TAG, "Unbinding service " + mIntent + " " + mUser);
        // Give it another chance next time it needs to be bound, out of kindness.
        mBindTryCount = 0;
        freeWrapper();
        if (mIsBound.get()) {
            try {
                mContext.unbindService(this);
            } catch (Exception e) {
                Log.e(TAG, "Failed to unbind service "
                        + mIntent.getComponent().flattenToShortString(), e);
            }
            mIsBound.set(false);
        }
    }

    @Override
    public void onServiceConnected(ComponentName name, IBinder service) {
        if (DEBUG) Log.d(TAG, "onServiceConnected " + name);
        if (mDebug) Log.d(TAG, "onServiceConnected " + name);
        // Got a connection, set the binding count to 0.
        mBindTryCount = 0;
        final QSTileServiceWrapper wrapper = new QSTileServiceWrapper(Stub.asInterface(service));
@@ -284,11 +291,17 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    }

    @Override
    public void onServiceDisconnected(ComponentName name) {
        if (DEBUG) Log.d(TAG, "onServiceDisconnected " + name);
    public void onBindingDied(ComponentName name) {
        if (mDebug) Log.d(TAG, "onBindingDied " + name);
        handleDeath();
    }

    @Override
    public void onServiceDisconnected(ComponentName name) {
        if (mDebug) Log.d(TAG, "onServiceDisconnected " + name);
        freeWrapper();
    }

    private void handlePendingMessages() {
        // This ordering is laid out manually to make sure we preserve the TileService
        // lifecycle.
@@ -298,35 +311,36 @@ public class TileLifecycleManager extends BroadcastReceiver implements
            mQueuedMessages.clear();
        }
        if (queue.contains(MSG_ON_ADDED)) {
            if (DEBUG) Log.d(TAG, "Handling pending onAdded");
            if (mDebug) Log.d(TAG, "Handling pending onAdded " + getComponent());
            onTileAdded();
        }
        if (mListening) {
            if (DEBUG) Log.d(TAG, "Handling pending onStartListening");
            if (mDebug) Log.d(TAG, "Handling pending onStartListening " + getComponent());
            onStartListening();
        }
        if (queue.contains(MSG_ON_CLICK)) {
            if (DEBUG) Log.d(TAG, "Handling pending onClick");
            if (mDebug) Log.d(TAG, "Handling pending onClick " + getComponent());
            if (!mListening) {
                Log.w(TAG, "Managed to get click on non-listening state...");
                Log.w(TAG, "Managed to get click on non-listening state... " + getComponent());
                // Skipping click since lost click privileges.
            } else {
                onClick(mClickBinder);
            }
        }
        if (queue.contains(MSG_ON_UNLOCK_COMPLETE)) {
            if (DEBUG) Log.d(TAG, "Handling pending onUnlockComplete");
            if (mDebug) Log.d(TAG, "Handling pending onUnlockComplete " + getComponent());
            if (!mListening) {
                Log.w(TAG, "Managed to get unlock on non-listening state...");
                Log.w(TAG,
                        "Managed to get unlock on non-listening state... " + getComponent());
                // Skipping unlock since lost click privileges.
            } else {
                onUnlockComplete();
            }
        }
        if (queue.contains(MSG_ON_REMOVED)) {
            if (DEBUG) Log.d(TAG, "Handling pending onRemoved");
            if (mDebug) Log.d(TAG, "Handling pending onRemoved " + getComponent());
            if (mListening) {
                Log.w(TAG, "Managed to get remove in listening state...");
                Log.w(TAG, "Managed to get remove in listening state... " + getComponent());
                onStopListening();
            }
            onTileRemoved();
@@ -340,28 +354,44 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    }

    public void handleDestroy() {
        if (DEBUG) Log.d(TAG, "handleDestroy");
        if (mDebug) Log.d(TAG, "handleDestroy");
        if (mPackageReceiverRegistered.get() || mUserReceiverRegistered.get()) {
            stopPackageListening();
        }
        mChangeListener = null;
    }

    /**
     * Handles a dead binder.
     *
     * It means that we need to clean up the binding (calling unbindService). After that, if we
     * are supposed to be bound, we will try to bind after some amount of time.
     */
    private void handleDeath() {
        if (mWrapper == null) return;
        freeWrapper();
        // Clearly not bound anymore
        mIsBound.set(false);
        if (!mBound.get()) return;
        if (DEBUG) Log.d(TAG, "handleDeath");
        if (checkComponentState()) {
        mExecutor.execute(() -> {
            if (!mIsBound.get()) {
                // If we are already not bound, don't do anything else.
                return;
            }
            // Clearly we shouldn't be bound anymore
            if (mDebug) Log.d(TAG, "handleDeath " + getComponent());
            // Binder died, make sure that we unbind. However, we don't want to call setBindService
            // as we still may want to rebind.
            unbindService();
            // If mBound is true (meaning that we should be bound), then reschedule binding for
            // later.
            if (mBound.get() && checkComponentState()) {
                if (isDeathRebindScheduled.compareAndSet(false, true)) {
                    mExecutor.executeDelayed(() -> {
                        // Only rebind if we are supposed to, but remove the scheduling anyway.
                        if (mBound.get()) {
                            setBindService(true);
                        }
                        isDeathRebindScheduled.set(false);
                    }, getRebindDelay());
                }
            }
        });
    }

    /**
@@ -379,7 +409,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
        } else {
            delay = mBindRetryDelay;
        }
        Log.i(TAG, "Rebinding with a delay=" + delay);
        if (mDebug) Log.i(TAG, "Rebinding with a delay=" + delay + " - " + getComponent());
        return delay;
    }

@@ -392,7 +422,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
    }

    private void startPackageListening() {
        if (DEBUG) Log.d(TAG, "startPackageListening");
        if (mDebug) Log.d(TAG, "startPackageListening " + getComponent());
        IntentFilter filter = new IntentFilter(Intent.ACTION_PACKAGE_ADDED);
        filter.addAction(Intent.ACTION_PACKAGE_CHANGED);
        filter.addDataScheme("package");
@@ -402,7 +432,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
                    this, mUser, filter, null, mHandler, Context.RECEIVER_EXPORTED);
        } catch (Exception ex) {
            mPackageReceiverRegistered.set(false);
            Log.e(TAG, "Could not register package receiver", ex);
            Log.e(TAG, "Could not register package receiver " + getComponent(), ex);
        }
        filter = new IntentFilter(Intent.ACTION_USER_UNLOCKED);
        try {
@@ -410,12 +440,12 @@ public class TileLifecycleManager extends BroadcastReceiver implements
            mBroadcastDispatcher.registerReceiverWithHandler(this, filter, mHandler, mUser);
        } catch (Exception ex) {
            mUserReceiverRegistered.set(false);
            Log.e(TAG, "Could not register unlock receiver", ex);
            Log.e(TAG, "Could not register unlock receiver " + getComponent(), ex);
        }
    }

    private void stopPackageListening() {
        if (DEBUG) Log.d(TAG, "stopPackageListening");
        if (mDebug) Log.d(TAG, "stopPackageListening " + getComponent());
        if (mUserReceiverRegistered.compareAndSet(true, false)) {
            mBroadcastDispatcher.unregisterReceiver(this);
        }
@@ -430,7 +460,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void onReceive(Context context, Intent intent) {
        if (DEBUG) Log.d(TAG, "onReceive: " + intent);
        if (mDebug) Log.d(TAG, "onReceive: " + intent);
        if (!Intent.ACTION_USER_UNLOCKED.equals(intent.getAction())) {
            Uri data = intent.getData();
            String pkgName = data.getEncodedSchemeSpecificPart();
@@ -446,7 +476,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
            if (mBound.get()) {
                // Trying to bind again will check the state of the package before bothering to
                // bind.
                if (DEBUG) Log.d(TAG, "Trying to rebind");
                if (mDebug) Log.d(TAG, "Trying to rebind " + getComponent());
                setBindService(true);
            }

@@ -458,7 +488,9 @@ public class TileLifecycleManager extends BroadcastReceiver implements
        try {
            ServiceInfo si = mPackageManagerAdapter.getServiceInfo(mIntent.getComponent(),
                    0, mUser.getIdentifier());
            if (DEBUG && si == null) Log.d(TAG, "Can't find component " + mIntent.getComponent());
            if (mDebug && si == null) {
                Log.d(TAG, "Can't find component " + mIntent.getComponent());
            }
            return si != null;
        } catch (RemoteException e) {
            // Shouldn't happen.
@@ -472,7 +504,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements
            mPackageManagerAdapter.getPackageInfoAsUser(packageName, 0, mUser.getIdentifier());
            return true;
        } catch (PackageManager.NameNotFoundException e) {
            if (DEBUG) {
            if (mDebug) {
                Log.d(TAG, "Package not available: " + packageName, e);
            } else {
                Log.d(TAG, "Package not available: " + packageName);
@@ -489,7 +521,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void onTileAdded() {
        if (DEBUG) Log.d(TAG, "onTileAdded");
        if (mDebug) Log.d(TAG, "onTileAdded " + getComponent());
        if (mWrapper == null || !mWrapper.onTileAdded()) {
            queueMessage(MSG_ON_ADDED);
            handleDeath();
@@ -498,7 +530,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void onTileRemoved() {
        if (DEBUG) Log.d(TAG, "onTileRemoved");
        if (mDebug) Log.d(TAG, "onTileRemoved " + getComponent());
        if (mWrapper == null || !mWrapper.onTileRemoved()) {
            queueMessage(MSG_ON_REMOVED);
            handleDeath();
@@ -507,7 +539,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void onStartListening() {
        if (DEBUG) Log.d(TAG, "onStartListening");
        if (mDebug) Log.d(TAG, "onStartListening " + getComponent());
        mListening = true;
        if (mWrapper != null && !mWrapper.onStartListening()) {
            handleDeath();
@@ -516,7 +548,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void onStopListening() {
        if (DEBUG) Log.d(TAG, "onStopListening");
        if (mDebug) Log.d(TAG, "onStopListening " + getComponent());
        mListening = false;
        if (mWrapper != null && !mWrapper.onStopListening()) {
            handleDeath();
@@ -525,7 +557,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void onClick(IBinder iBinder) {
        if (DEBUG) Log.d(TAG, "onClick " + iBinder + " " + mUser);
        if (mDebug) Log.d(TAG, "onClick " + iBinder + " " + getComponent() + " " + mUser);
        if (mWrapper == null || !mWrapper.onClick(iBinder)) {
            mClickBinder = iBinder;
            queueMessage(MSG_ON_CLICK);
@@ -535,7 +567,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void onUnlockComplete() {
        if (DEBUG) Log.d(TAG, "onUnlockComplete");
        if (mDebug) Log.d(TAG, "onUnlockComplete " + getComponent());
        if (mWrapper == null || !mWrapper.onUnlockComplete()) {
            queueMessage(MSG_ON_UNLOCK_COMPLETE);
            handleDeath();
@@ -550,7 +582,7 @@ public class TileLifecycleManager extends BroadcastReceiver implements

    @Override
    public void binderDied() {
        if (DEBUG) Log.d(TAG, "binderDeath");
        if (mDebug) Log.d(TAG, "binderDeath " + getComponent());
        handleDeath();
    }

+53 −4
Original line number Diff line number Diff line
@@ -29,6 +29,7 @@ import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.Mockito.any;
import static org.mockito.Mockito.anyInt;
import static org.mockito.Mockito.anyString;
import static org.mockito.Mockito.clearInvocations;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.never;
@@ -300,8 +301,10 @@ public class TileLifecycleManagerTest extends SysuiTestCase {
        mStateManager.onStartListening();
        mStateManager.executeSetBindService(true);
        mExecutor.runAllReady();
        mStateManager.onServiceDisconnected(mTileServiceComponentName);
        mStateManager.onBindingDied(mTileServiceComponentName);
        mExecutor.runAllReady();
        mClock.advanceTime(5000);
        mExecutor.runAllReady();

        // Two calls: one for the first bind, one for the restart.
        verifyBind(2);
@@ -318,19 +321,65 @@ public class TileLifecycleManagerTest extends SysuiTestCase {
        mStateManager.onStartListening();
        mStateManager.executeSetBindService(true);
        mExecutor.runAllReady();
        mStateManager.onServiceDisconnected(mTileServiceComponentName);
        verify(mMockTileService, times(1)).onStartListening();
        mStateManager.onBindingDied(mTileServiceComponentName);
        mExecutor.runAllReady();

        // Longer delay than a regular one
        mClock.advanceTime(5000);
        verifyBind(1);
        verify(mMockTileService, times(1)).onStartListening();
        mExecutor.runAllReady();

        assertFalse(mContext.isBound(mTileServiceComponentName));

        mClock.advanceTime(20000);
        mExecutor.runAllReady();
        // Two calls: one for the first bind, one for the restart.
        verifyBind(2);
        verify(mMockTileService, times(2)).onStartListening();
    }

    @Test
    public void testOnServiceDisconnectedDoesnUnbind_doesntForwardToBinder() throws Exception {
        mStateManager.executeSetBindService(true);
        mExecutor.runAllReady();

        mStateManager.onStartListening();
        verify(mMockTileService).onStartListening();

        clearInvocations(mMockTileService);
        mStateManager.onServiceDisconnected(mTileServiceComponentName);
        mExecutor.runAllReady();

        mStateManager.onStartListening();
        verify(mMockTileService, never()).onStartListening();
    }

    @Test
    public void testKillProcessLowMemory_unbound_doesntBindAgain() throws Exception {
        doAnswer(invocation -> {
            ActivityManager.MemoryInfo memoryInfo = invocation.getArgument(0);
            memoryInfo.lowMemory = true;
            return null;
        }).when(mActivityManager).getMemoryInfo(any());
        mStateManager.onStartListening();
        mStateManager.executeSetBindService(true);
        mExecutor.runAllReady();
        verifyBind(1);
        verify(mMockTileService, times(1)).onStartListening();

        mStateManager.onBindingDied(mTileServiceComponentName);
        mExecutor.runAllReady();

        clearInvocations(mMockTileService);
        mStateManager.executeSetBindService(false);
        mExecutor.runAllReady();
        mClock.advanceTime(30000);
        mExecutor.runAllReady();

        verifyBind(0);
        verify(mMockTileService, never()).onStartListening();
    }

    @Test
    public void testToggleableTile() throws Exception {
        assertTrue(mStateManager.isToggleableTile());