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

Commit 36c998c6 authored by Christopher Tate's avatar Christopher Tate
Browse files

Prevent double teardown of service connections 

Asynchronicities in activity teardown -> service connection teardown
introduced a race in which the teardown could race with new service
bindings to "the same" service instance, and then wind up attempting to
shut down a new, valid instance inappropriately.  Fixed by making sure
to clear the "what needs to be torn down" bookkeeping as part of the
act of doing that teardown, removing the possibility for stale state.

Fixes: 131029480
Test: manual
Test: atest CtsAppTestCases
Change-Id: I33a63f524d147ff6ec97dd3efb0127dcace8bf3c
parent a11f7912

core/java/android/app/ActivityManagerInternal.java

+1 −1
Original line number Diff line number Diff line
@@ -278,7 +278,7 @@ public abstract class ActivityManagerInternal { 
            String resolvedType, boolean fgRequired, String callingPackage, int userId,

            boolean allowBackgroundActivityStarts) throws TransactionTooLargeException;



    public abstract void disconnectActivityFromServices(Object connectionHolder);

    public abstract void disconnectActivityFromServices(Object connectionHolder, Object conns);

    public abstract void cleanUpServices(int userId, ComponentName component, Intent baseIntent);

    public abstract ActivityInfo getActivityInfoForUser(ActivityInfo aInfo, int userId);

    public abstract void ensureBootCompleted();

services/core/java/com/android/server/am/ActivityManagerService.java

+12 −5
Original line number Diff line number Diff line
@@ -18156,13 +18156,20 @@ public class ActivityManagerService extends IActivityManager.Stub 
            }

        }













        @Override













        public void disconnectActivityFromServices(Object connectionHolder) {













            synchronized(ActivityManagerService.this) {













                final ActivityServiceConnectionsHolder c =













        // The arguments here are untyped because the base ActivityManagerInternal class













        // doesn't have compile-time visiblity into ActivityServiceConnectionHolder or













        // ConnectionRecord.













        @Override













        public void disconnectActivityFromServices(Object connectionHolder, Object conns) {













            // 'connectionHolder' is an untyped ActivityServiceConnectionsHolder













            // 'conns' is an untyped HashSet<ConnectionRecord>













            final ActivityServiceConnectionsHolder holder =















                    (ActivityServiceConnectionsHolder) connectionHolder;













                c.forEachConnection(cr -> mServices.removeConnectionLocked(













                        (ConnectionRecord) cr, null, c));













            final HashSet<ConnectionRecord> toDisconnect = (HashSet<ConnectionRecord>) conns;













            synchronized(ActivityManagerService.this) {













                for (ConnectionRecord cr : toDisconnect) {













                    mServices.removeConnectionLocked(cr, null, holder);













                }















            }















        }







































services/core/java/com/android/server/wm/ActivityServiceConnectionsHolder.java



+7
−1




























Original line number
Diff line number
Diff line








@@ -101,7 +101,13 @@ public class ActivityServiceConnectionsHolder<T> {













        if (mConnections == null || mConnections.isEmpty()) {















            return;















        }













        mService.mH.post(() -> mService.mAmInternal.disconnectActivityFromServices(this));













        // Capture and null out mConnections, to guarantee that we process













        // disconnect of these specific connections exactly once even if













        // we're racing with rapid activity lifecycle churn and this













        // method is invoked more than once on this object.













        final Object disc = mConnections;













        mConnections = null;













        mService.mH.post(() -> mService.mAmInternal.disconnectActivityFromServices(this, disc));















    }































    public void dump(PrintWriter pw, String prefix) {