android: Android 10 isolated service bug workaround

Android 10 has a bug that UID used for non-primary user cannot
be freed correctly, eventually exhausting the pool of UIDs for
isolated services. There is a global pool of 1000 UIDs, and each
app zygote has a smaller pool of 100; the bug appplies to both cases.
The leaked UID in the app zygote pool are released when the zygote
is killed; leaked UIDs in the global pool are released when the
device is rebooted. So way to slightly delay until the device needs
to be rebooted is to use up the app zygote pool first before using
the non-zygote global pool.

Add a workaround allocator that's only used under secondary user
on Android 10. It will try to bind to a new service from the app
zygote. If bind fails, which happens if the UID pool is exhausted,
then try using the non app zygote service.

Bug: 1035432
Change-Id: I07cba74a21076e4cbc9342705f7b69b97e011c20
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2001998
Commit-Queue: Bo <boliu@chromium.org>
Reviewed-by: Tommy Nyquist <nyquist@chromium.org>
Reviewed-by: ssid <ssid@chromium.org>
Cr-Commit-Position: refs/heads/master@{#733829}

base/android/java/src/org/chromium/base/process_launcher/ChildConnectionAllocator.java

@@ -33,6 +33,8 @@ import java.util.Queue;
  */
 public abstract class ChildConnectionAllocator {
     private static final String TAG = "ChildConnAllocator";
+    private static final String ZYGOTE_SUFFIX = "0";
+    private static final String NON_ZYGOTE_SUFFIX = "1";
 
     /** Factory interface. Used by tests to specialize created connections. */
     @VisibleForTesting
@@ -72,7 +74,7 @@ public abstract class ChildConnectionAllocator {
     /* package */ final String mServiceClassName;
     /* package */ final boolean mBindToCaller;
     /* package */ final boolean mBindAsExternalService;
-    private final boolean mUseStrongBinding;
+    /* package */ final boolean mUseStrongBinding;
 
     /* package */ ConnectionFactory mConnectionFactory = new ConnectionFactoryImpl();
 
@@ -125,9 +127,19 @@ public abstract class ChildConnectionAllocator {
             String serviceClassName, boolean bindToCaller, boolean bindAsExternalService,
             boolean useStrongBinding) {
         checkServiceExists(context, packageName, serviceClassName);
+        if (Build.VERSION.SDK_INT == 29) {
+            UserManager userManager =
+                    (UserManager) ContextUtils.getApplicationContext().getSystemService(
+                            Context.USER_SERVICE);
+            if (!ApiHelperForM.isSystemUser(userManager)) {
+                return new Android10WorkaroundAllocatorImpl(launcherHandler, freeSlotCallback,
+                        packageName, serviceClassName, bindToCaller, bindAsExternalService,
+                        useStrongBinding, MAX_VARIABLE_ALLOCATED);
+            }
+        }
         return new VariableSizeAllocatorImpl(launcherHandler, freeSlotCallback, packageName,
-                serviceClassName, bindToCaller, bindAsExternalService, useStrongBinding,
-                MAX_VARIABLE_ALLOCATED);
+                serviceClassName + ZYGOTE_SUFFIX, bindToCaller, bindAsExternalService,
+                useStrongBinding, MAX_VARIABLE_ALLOCATED);
     }
 
     /**
@@ -149,7 +161,17 @@ public abstract class ChildConnectionAllocator {
             boolean bindToCaller, boolean bindAsExternalService, boolean useStrongBinding,
             int maxAllocated) {
         return new VariableSizeAllocatorImpl(launcherHandler, freeSlotCallback, packageName,
-                serviceClassName + "0", bindToCaller, bindAsExternalService, useStrongBinding,
+                serviceClassName + ZYGOTE_SUFFIX, bindToCaller, bindAsExternalService,
+                useStrongBinding, maxAllocated);
+    }
+
+    @VisibleForTesting
+    public static Android10WorkaroundAllocatorImpl createWorkaroundForTesting(
+            Handler launcherHandler, String packageName, Runnable freeSlotCallback,
+            String serviceClassName, boolean bindToCaller, boolean bindAsExternalService,
+            boolean useStrongBinding, int maxAllocated) {
+        return new Android10WorkaroundAllocatorImpl(launcherHandler, freeSlotCallback, packageName,
+                serviceClassName, bindToCaller, bindAsExternalService, useStrongBinding,
                 maxAllocated);
     }
 
@@ -171,9 +193,6 @@ public abstract class ChildConnectionAllocator {
             final ChildProcessConnection.ServiceCallback serviceCallback) {
         assert isRunningOnLauncherThread();
 
-        ChildProcessConnection connection = doAllocate(context, serviceBundle);
-        if (connection == null) return null;
-
         // Wrap the service callbacks so that:
         // - we can intercept onChildProcessDied and clean-up connections
         // - the callbacks are actually posted so that this method will return before the callbacks
@@ -238,8 +257,7 @@ public abstract class ChildConnectionAllocator {
                     }
                 };
 
-        connection.start(mUseStrongBinding, serviceCallbackWrapper);
-        return connection;
+        return doAllocate(context, serviceBundle, serviceCallbackWrapper);
     }
 
     /** Free connection allocated by this allocator. */
@@ -280,7 +298,8 @@ public abstract class ChildConnectionAllocator {
         return mLauncherHandler.getLooper() == Looper.myLooper();
     }
 
-    /* package */ abstract ChildProcessConnection doAllocate(Context context, Bundle serviceBundle);
+    /* package */ abstract ChildProcessConnection doAllocate(Context context, Bundle serviceBundle,
+            ChildProcessConnection.ServiceCallback serviceCallback);
     /* package */ abstract void doFree(ChildProcessConnection connection);
 
     /** Implementation class accessed directly by tests. */
@@ -307,7 +326,8 @@ public abstract class ChildConnectionAllocator {
         }
 
         @Override
-        /* package */ ChildProcessConnection doAllocate(Context context, Bundle serviceBundle) {
+        /* package */ ChildProcessConnection doAllocate(Context context, Bundle serviceBundle,
+                ChildProcessConnection.ServiceCallback serviceCallback) {
             if (mFreeConnectionIndices.isEmpty()) {
                 Log.d(TAG, "Ran out of services to allocate.");
                 return null;
@@ -322,6 +342,7 @@ public abstract class ChildConnectionAllocator {
             mChildProcessConnections[slot] = connection;
             Log.d(TAG, "Allocator allocated and bound a connection, name: %s, slot: %d",
                     mServiceClassName, slot);
+            connection.start(mUseStrongBinding, serviceCallback);
             return connection;
         }
 
@@ -374,34 +395,37 @@ public abstract class ChildConnectionAllocator {
         private final ArraySet<ChildProcessConnection> mAllocatedConnections = new ArraySet<>();
         private int mNextInstance;
 
-        private static String getServiceSuffix() {
-            // Android Q has a bug in its app zygote implementation under secondary user (eg in a
-            // work profile). See crbug.com/1035432 for details. Disable using the app zygote in
-            // that case by using a non '0' suffix which is the only service entry that enables
-            // app zygote.
-            if (Build.VERSION.SDK_INT == 29) {
-                UserManager userManager =
-                        (UserManager) ContextUtils.getApplicationContext().getSystemService(
-                                Context.USER_SERVICE);
-                if (!ApiHelperForM.isSystemUser(userManager)) {
-                    return "1";
-                }
-            }
-            return "0";
-        }
-
+        // Note |serviceClassName| includes the service suffix.
         private VariableSizeAllocatorImpl(Handler launcherHandler, Runnable freeSlotCallback,
                 String packageName, String serviceClassName, boolean bindToCaller,
                 boolean bindAsExternalService, boolean useStrongBinding, int maxAllocated) {
-            super(launcherHandler, freeSlotCallback, packageName,
-                    serviceClassName + getServiceSuffix(), bindToCaller, bindAsExternalService,
-                    useStrongBinding);
+            super(launcherHandler, freeSlotCallback, packageName, serviceClassName, bindToCaller,
+                    bindAsExternalService, useStrongBinding);
             assert maxAllocated > 0;
             mMaxAllocated = maxAllocated;
         }
 
         @Override
-        /* package */ ChildProcessConnection doAllocate(Context context, Bundle serviceBundle) {
+        /* package */ ChildProcessConnection doAllocate(Context context, Bundle serviceBundle,
+                ChildProcessConnection.ServiceCallback serviceCallback) {
+            ChildProcessConnection connection = allocate(context, serviceBundle);
+            if (connection == null) return null;
+            mAllocatedConnections.add(connection);
+            connection.start(mUseStrongBinding, serviceCallback);
+            return connection;
+        }
+
+        /* package */ ChildProcessConnection tryAllocate(Context context, Bundle serviceBundle,
+                ChildProcessConnection.ServiceCallback serviceCallback) {
+            ChildProcessConnection connection = allocate(context, serviceBundle);
+            if (connection == null) return null;
+            boolean startResult = connection.tryStart(mUseStrongBinding, serviceCallback);
+            if (!startResult) return null;
+            mAllocatedConnections.add(connection);
+            return connection;
+        }
+
+        private ChildProcessConnection allocate(Context context, Bundle serviceBundle) {
             if (mAllocatedConnections.size() >= mMaxAllocated) {
                 Log.d(TAG, "Ran out of UIDs to allocate.");
                 return null;
@@ -413,13 +437,17 @@ public abstract class ChildConnectionAllocator {
                     mConnectionFactory.createConnection(context, serviceName, mBindToCaller,
                             mBindAsExternalService, serviceBundle, instanceName);
             assert connection != null;
-            mAllocatedConnections.add(connection);
             return connection;
         }
 
         @Override
         /* package */ void doFree(ChildProcessConnection connection) {
-            mAllocatedConnections.remove(connection);
+            boolean result = mAllocatedConnections.remove(connection);
+            assert result;
+        }
+
+        /* package */ boolean wasConnectionAllocated(ChildProcessConnection connection) {
+            return mAllocatedConnections.contains(connection);
         }
 
         @Override
@@ -437,4 +465,76 @@ public abstract class ChildConnectionAllocator {
             return mAllocatedConnections.size() > 0;
         }
     }
+
+    /**
+     * Workaround allocator for Android 10 bug.
+     * Android 10 has a bug that UID used for non-primary user cannot be freed correctly,
+     * eventually exhausting the pool of UIDs for isolated services. There is a global pool of
+     * 1000 UIDs, and each app zygote has a smaller pool of 100; the bug appplies to both cases.
+     * The leaked UID in the app zygote pool are released when the zygote is killed; leaked UIDs in
+     * the global pool are released when the device is rebooted. So way to slightly delay until the
+     * device needs to be rebooted is to use up the app zygote pool first before using the
+     * non-zygote global pool.
+     */
+    private static class Android10WorkaroundAllocatorImpl extends ChildConnectionAllocator {
+        private final VariableSizeAllocatorImpl mZygoteAllocator;
+        private final VariableSizeAllocatorImpl mNonZygoteAllocator;
+
+        private Android10WorkaroundAllocatorImpl(Handler launcherHandler, Runnable freeSlotCallback,
+                String packageName, String serviceClassName, boolean bindToCaller,
+                boolean bindAsExternalService, boolean useStrongBinding, int maxAllocated) {
+            super(launcherHandler, freeSlotCallback, packageName, serviceClassName, bindToCaller,
+                    bindAsExternalService, useStrongBinding);
+            mZygoteAllocator = new VariableSizeAllocatorImpl(launcherHandler, freeSlotCallback,
+                    packageName, serviceClassName + ZYGOTE_SUFFIX, bindToCaller,
+                    bindAsExternalService, useStrongBinding, maxAllocated);
+            mNonZygoteAllocator = new VariableSizeAllocatorImpl(launcherHandler, freeSlotCallback,
+                    packageName, serviceClassName + NON_ZYGOTE_SUFFIX, bindToCaller,
+                    bindAsExternalService, useStrongBinding, maxAllocated);
+        }
+
+        @Override
+        /* package */ ChildProcessConnection doAllocate(Context context, Bundle serviceBundle,
+                ChildProcessConnection.ServiceCallback serviceCallback) {
+            ChildProcessConnection connection =
+                    mZygoteAllocator.tryAllocate(context, serviceBundle, serviceCallback);
+            if (connection != null) return connection;
+            return mNonZygoteAllocator.doAllocate(context, serviceBundle, serviceCallback);
+        }
+
+        @Override
+        /* package */ void doFree(ChildProcessConnection connection) {
+            if (mZygoteAllocator.wasConnectionAllocated(connection)) {
+                mZygoteAllocator.doFree(connection);
+            } else if (mNonZygoteAllocator.wasConnectionAllocated(connection)) {
+                mNonZygoteAllocator.doFree(connection);
+            } else {
+                assert false;
+            }
+        }
+
+        @Override
+        public int getNumberOfServices() {
+            return -1;
+        }
+
+        @Override
+        public int allocatedConnectionsCountForTesting() {
+            return mZygoteAllocator.allocatedConnectionsCountForTesting()
+                    + mNonZygoteAllocator.allocatedConnectionsCountForTesting();
+        }
+
+        @Override
+        public boolean anyConnectionAllocated() {
+            return mZygoteAllocator.anyConnectionAllocated()
+                    || mNonZygoteAllocator.anyConnectionAllocated();
+        }
+
+        @Override
+        public void setConnectionFactoryForTesting(ConnectionFactory connectionFactory) {
+            super.setConnectionFactoryForTesting(connectionFactory);
+            mZygoteAllocator.setConnectionFactoryForTesting(connectionFactory);
+            mNonZygoteAllocator.setConnectionFactoryForTesting(connectionFactory);
+        }
+    }
 }

base/android/java/src/org/chromium/base/process_launcher/ChildProcessConnection.java

@@ -416,6 +416,27 @@ public class ChildProcessConnection {
         }
     }
 
+    // This is the same as start, but returns a boolean whether bind succeeded. Also on failure,
+    // no method is called on |serviceCallback| so the allocation can be tried again. This is
+    // package private and is meant to be used by Android10WorkaroundAllocatorImpl. See comment
+    // there for details.
+    boolean tryStart(boolean useStrongBinding, ServiceCallback serviceCallback) {
+        try {
+            TraceEvent.begin("ChildProcessConnection.tryStart");
+            assert isRunningOnLauncherThread();
+            assert mConnectionParams
+                    == null : "setupConnection() called before start() in ChildProcessConnection.";
+
+            if (!bind(useStrongBinding)) {
+                return false;
+            }
+            mServiceCallback = serviceCallback;
+        } finally {
+            TraceEvent.end("ChildProcessConnection.tryStart");
+        }
+        return true;
+    }
+
     /**
      * Call bindService again on this connection. This must be called while connection is already
      * bound. This is useful for controlling the recency of this connection, and also for updating

base/android/junit/src/org/chromium/base/process_launcher/ChildConnectionAllocatorTest.java

@@ -138,6 +138,7 @@ public class ChildConnectionAllocatorTest {
 
     private ChildConnectionAllocator.FixedSizeAllocatorImpl mAllocator;
     private ChildConnectionAllocator mVariableSizeAllocator;
+    private ChildConnectionAllocator mWorkaroundAllocator;
 
     @Before
     public void setUp() {
@@ -153,6 +154,12 @@ public class ChildConnectionAllocatorTest {
                 true /* bindTocall */, false /* bindAsExternalService */,
                 false /* useStrongBinding */, 10);
         mVariableSizeAllocator.setConnectionFactoryForTesting(mTestConnectionFactory);
+
+        mWorkaroundAllocator = ChildConnectionAllocator.createWorkaroundForTesting(new Handler(),
+                TEST_PACKAGE_NAME, null /* freeSlotCallback */, "AllocatorTest",
+                true /* bindTocall */, false /* bindAsExternalService */,
+                false /* useStrongBinding */, 10);
+        mWorkaroundAllocator.setConnectionFactoryForTesting(mTestConnectionFactory);
     }
 
     @Test
@@ -211,6 +218,16 @@ public class ChildConnectionAllocatorTest {
         doTestQueueAllocation(mVariableSizeAllocator, freeConnectionCallback);
     }
 
+    @Test
+    @Feature({"ProcessManagement"})
+    public void testQueueAllocationWorkaround() {
+        Runnable freeConnectionCallback = mock(Runnable.class);
+        mWorkaroundAllocator = ChildConnectionAllocator.createWorkaroundForTesting(new Handler(),
+                TEST_PACKAGE_NAME, freeConnectionCallback, "AllocatorTest", true /* bindToCaller */,
+                false /* bindAsExternalService */, false /* useStrongBinding */, 1);
+        doTestQueueAllocation(mWorkaroundAllocator, freeConnectionCallback);
+    }
+
     private void doTestQueueAllocation(
             ChildConnectionAllocator allocator, Runnable freeConnectionCallback) {
         allocator.setConnectionFactoryForTesting(mTestConnectionFactory);
@@ -301,6 +318,13 @@ public class ChildConnectionAllocatorTest {
                 false /* onChildStartFailed */, false /* onChildProcessDied */);
     }
 
+    @Test
+    @Feature({"ProcessManagement"})
+    public void testOnChildStartedCallbackWorkaround() {
+        runTestWithConnectionCallbacks(mWorkaroundAllocator, true /* onChildStarted */,
+                false /* onChildStartFailed */, false /* onChildProcessDied */);
+    }
+
     @Test
     @Feature({"ProcessManagement"})
     public void testOnChildStartFailedCallback() {
@@ -315,6 +339,13 @@ public class ChildConnectionAllocatorTest {
                 true /* onChildStartFailed */, false /* onChildProcessDied */);
     }
 
+    @Test
+    @Feature({"ProcessManagement"})
+    public void testOnChildStartFailedCallbackWorkaround() {
+        runTestWithConnectionCallbacks(mWorkaroundAllocator, false /* onChildStarted */,
+                true /* onChildStartFailed */, false /* onChildProcessDied */);
+    }
+
     @Test
     @Feature({"ProcessManagement"})
     public void testOnChildProcessDiedCallback() {
@@ -329,6 +360,13 @@ public class ChildConnectionAllocatorTest {
                 false /* onChildStartFailed */, true /* onChildProcessDied */);
     }
 
+    @Test
+    @Feature({"ProcessManagement"})
+    public void testOnChildProcessDiedCallbackWorkaround() {
+        runTestWithConnectionCallbacks(mWorkaroundAllocator, false /* onChildStarted */,
+                false /* onChildStartFailed */, true /* onChildProcessDied */);
+    }
+
     /**
      * Tests that the allocator clears the connection when it fails to bind/process dies.
      */
@@ -389,6 +427,12 @@ public class ChildConnectionAllocatorTest {
         testFreeConnection(mVariableSizeAllocator, FREE_CONNECTION_TEST_CALLBACK_START_FAILED);
     }
 
+    @Test
+    @Feature({"ProcessManagement"})
+    public void testFreeConnectionOnChildStartFailedWorkaround() {
+        testFreeConnection(mWorkaroundAllocator, FREE_CONNECTION_TEST_CALLBACK_START_FAILED);
+    }
+
     @Test
     @Feature({"ProcessManagement"})
     public void testFreeConnectionOnChildProcessDied() {
@@ -400,4 +444,10 @@ public class ChildConnectionAllocatorTest {
     public void testFreeConnectionOnChildProcessDiedVariableSize() {
         testFreeConnection(mVariableSizeAllocator, FREE_CONNECTION_TEST_CALLBACK_PROCESS_DIED);
     }
+
+    @Test
+    @Feature({"ProcessManagement"})
+    public void testFreeConnectionOnChildProcessDiedWorkaround() {
+        testFreeConnection(mWorkaroundAllocator, FREE_CONNECTION_TEST_CALLBACK_PROCESS_DIED);
+    }
 }