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

Commit c9e4e76b authored by David Samuelson's avatar David Samuelson
Browse files

Improve handling of GameService when bound processes die. 

 - Add callbacks to observer service lifecycle events to ServiceConnector.
 - Leverage callbacks to avoid re-binding to GameService in order to
   send disconnected event
 - Leverage callbacks to detect when GameSessionService process dies and
   destroy all GameSessions.

Test: Manually verified GameSession overlays are removed when
GameSession process crashes. Added unit test covering behavior.
Bug: 220204229
Bug: 215599171

Change-Id: Idda96f221e40c88f0f80c95bea94556275da5166
parent 7a53fdf6

core/java/com/android/internal/infra/ServiceConnector.java

+71 −3
Original line number Diff line number Diff line
@@ -146,6 +146,15 @@ public interface ServiceConnector<I extends IInterface> { 
     */

    void unbind();



    /**

     * Registers a {@link ServiceLifecycleCallbacks callbacks} to be invoked when the lifecycle

     * of the managed service changes.

     *

     * @param callbacks The callbacks that will be run, or {@code null} to clear the existing

     *                 callbacks.

     */

    void setServiceLifecycleCallbacks(@Nullable ServiceLifecycleCallbacks<I> callbacks);



    /**

     * A request to be run when the service is

     * {@link ServiceConnection#onServiceConnected connected}.
@@ -189,6 +198,32 @@ public interface ServiceConnector<I extends IInterface> { 
        }

    }



    /**

     * Collection of callbacks invoked when the lifecycle of the service changes.

     *

     * @param <II> the type of the {@link IInterface ipc interface} for the remote service

     * @see ServiceConnector#setServiceLifecycleCallbacks(ServiceLifecycleCallbacks)

     */

    interface ServiceLifecycleCallbacks<II extends IInterface> {

        /**

         * Called when the service has just connected and before any queued jobs are run.

         */

        default void onConnected(@NonNull II service) {}



        /**

         * Called just before the service is disconnected and unbound.

         */

        default void onDisconnected(@NonNull II service) {}



        /**

         * Called when the service Binder has died.

         *

         * In cases where {@link #onBinderDied()} is invoked the service becomes unbound without

         * a callback to {@link #onDisconnected(IInterface)}.

         */

        default void onBinderDied() {}

    }





    /**

     * Implementation of {@link ServiceConnector}
@@ -230,6 +265,8 @@ public interface ServiceConnector<I extends IInterface> { 
        private final @NonNull Handler mHandler;

        protected final @NonNull Executor mExecutor;



        @Nullable

        private volatile ServiceLifecycleCallbacks<I> mServiceLifecycleCallbacks = null;

        private volatile I mService = null;

        private boolean mBinding = false;

        private boolean mUnbinding = false;
@@ -330,6 +367,19 @@ public interface ServiceConnector<I extends IInterface> { 
            }

        }



        private void dispatchOnServiceConnectionStatusChanged(

                @NonNull I service, boolean isConnected) {

            ServiceLifecycleCallbacks<I> serviceLifecycleCallbacks = mServiceLifecycleCallbacks;

            if (serviceLifecycleCallbacks != null) {

                if (isConnected) {

                    serviceLifecycleCallbacks.onConnected(service);

                } else {

                    serviceLifecycleCallbacks.onDisconnected(service);

                }

            }

            onServiceConnectionStatusChanged(service, isConnected);

        }



        /**

         * Called when the service just connected or is about to disconnect

         */
@@ -504,12 +554,17 @@ public interface ServiceConnector<I extends IInterface> { 
            mHandler.post(this::unbindJobThread);

        }



        @Override

        public void setServiceLifecycleCallbacks(@Nullable ServiceLifecycleCallbacks<I> callbacks) {

            mServiceLifecycleCallbacks = callbacks;

        }



        void unbindJobThread() {

            cancelTimeout();

            I service = mService;

            boolean wasBound = service != null;

            if (wasBound) {

                onServiceConnectionStatusChanged(service, false);

                dispatchOnServiceConnectionStatusChanged(service, false);

                mContext.unbindService(mServiceConnection);

                service.asBinder().unlinkToDeath(this, 0);

                mService = null;
@@ -560,7 +615,7 @@ public interface ServiceConnector<I extends IInterface> { 
            } catch (RemoteException e) {

                Log.e(LOG_TAG, "onServiceConnected " + name + ": ", e);

            }

            onServiceConnectionStatusChanged(service, true);

            dispatchOnServiceConnectionStatusChanged(service, true);

            processQueue();

        }
@@ -572,7 +627,7 @@ public interface ServiceConnector<I extends IInterface> { 
            mBinding = true;

            I service = mService;

            if (service != null) {

                onServiceConnectionStatusChanged(service, false);

                dispatchOnServiceConnectionStatusChanged(service, false);

                mService = null;

            }

        }
@@ -592,6 +647,14 @@ public interface ServiceConnector<I extends IInterface> { 
            }

            mService = null;

            unbind();

            dispatchOnBinderDied();

        }



        private void dispatchOnBinderDied() {

            ServiceLifecycleCallbacks<I> serviceLifecycleCallbacks = mServiceLifecycleCallbacks;

            if (serviceLifecycleCallbacks != null) {

                serviceLifecycleCallbacks.onBinderDied();

            }

        }



        @Override
@@ -764,5 +827,10 @@ public interface ServiceConnector<I extends IInterface> { 


        @Override

        public void unbind() {}



        @Override

        public void setServiceLifecycleCallbacks(@Nullable ServiceLifecycleCallbacks<T> callbacks) {

            // Do nothing.

        }

    }

}

core/tests/coretests/AndroidManifest.xml

+10 −0
Original line number Diff line number Diff line
@@ -1494,6 +1494,16 @@ 
            android:permission="android.permission.BIND_SETTINGS_SUGGESTIONS_SERVICE"

            android:exported="true"/>



        <service

            android:name="com.android.internal.infra.ServiceConnectorTest$TestService"

            android:process=":ServiceConnectorRemoteService"

            android:exported="true">

            <intent-filter>

                <action android:name="android.intent.action.BIND_TEST_SERVICE" />

                <category android:name="android.intent.category.DEFAULT" />

            </intent-filter>

        </service>



        <provider android:name="android.app.activity.LocalProvider"

                android:authorities="com.android.frameworks.coretests.LocalProvider">

            <meta-data android:name="com.android.frameworks.coretests.string" android:value="foo" />

core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/ITestServiceConnectorService.aidl

0 → 100644
+21 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.frameworks.coretests.aidl;



interface ITestServiceConnectorService {

    void crashProcess();

}

core/tests/coretests/src/com/android/internal/infra/ServiceConnectorTest.java

0 → 100644
+242 −0
Original line number Diff line number Diff line 
/*

 * Copyright (C) 2022 The Android Open Source Project

 *

 * Licensed under the Apache License, Version 2.0 (the "License");

 * you may not use this file except in compliance with the License.

 * You may obtain a copy of the License at

 *

 *      http://www.apache.org/licenses/LICENSE-2.0

 *

 * Unless required by applicable law or agreed to in writing, software

 * distributed under the License is distributed on an "AS IS" BASIS,

 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

 * See the License for the specific language governing permissions and

 * limitations under the License.

 */



package com.android.internal.infra;



import static com.google.common.truth.Truth.assertThat;



import static java.util.Objects.requireNonNull;



import android.annotation.Nullable;

import android.app.Service;

import android.content.Context;

import android.content.Intent;

import android.os.IBinder;

import android.os.Process;

import android.os.UserHandle;



import androidx.annotation.NonNull;

import androidx.test.platform.app.InstrumentationRegistry;

import androidx.test.runner.AndroidJUnit4;



import com.android.frameworks.coretests.aidl.ITestServiceConnectorService;

import com.android.internal.infra.ServiceConnectorTest.CapturingServiceLifecycleCallbacks.ServiceLifeCycleEvent;



import org.junit.Before;

import org.junit.Test;

import org.junit.runner.RunWith;



import java.util.ArrayList;

import java.util.concurrent.CancellationException;

import java.util.concurrent.ExecutionException;

import java.util.concurrent.TimeUnit;

import java.util.concurrent.TimeoutException;



/**

 * Unit tests for {@link ServiceConnector}

 */

@RunWith(AndroidJUnit4.class)

public class ServiceConnectorTest {



    private final CapturingServiceLifecycleCallbacks mCapturingServiceLifecycleCallbacks =

            new CapturingServiceLifecycleCallbacks();

    private ServiceConnector<ITestServiceConnectorService> mServiceConnector;



    @Before

    public void setup() {

        Context context = InstrumentationRegistry.getInstrumentation().getContext();

        Intent testServiceConnectorServiceIntent = new Intent(TestService.ACTION_TEST_SERVICE);

        testServiceConnectorServiceIntent.setPackage(context.getPackageName());



        ServiceConnector.Impl<ITestServiceConnectorService> serviceConnector =

                new ServiceConnector.Impl<ITestServiceConnectorService>(

                        context,

                        testServiceConnectorServiceIntent,

                        /* bindingFlags= */ 0,

                        UserHandle.myUserId(),

                        ITestServiceConnectorService.Stub::asInterface);

        serviceConnector.setServiceLifecycleCallbacks(mCapturingServiceLifecycleCallbacks);

        mServiceConnector = serviceConnector;

    }



    @Test

    public void connect_invokesLifecycleCallbacks() throws Exception {

        connectAndWaitForDone();



        assertThat(mCapturingServiceLifecycleCallbacks.getCapturedLifecycleEvents())

                .containsExactly(ServiceLifeCycleEvent.ON_CONNECTED)

                .inOrder();

    }



    @Test

    public void connect_alreadyConnected_invokesLifecycleCallbacksOnce() throws Exception {

        connectAndWaitForDone();

        connectAndWaitForDone();



        assertThat(mCapturingServiceLifecycleCallbacks.getCapturedLifecycleEvents())

                .containsExactly(ServiceLifeCycleEvent.ON_CONNECTED)

                .inOrder();

    }



    @Test

    public void unbind_neverConnected_noLifecycleCallbacks() {

        unbindAndWaitForDone();



        assertThat(mCapturingServiceLifecycleCallbacks.getCapturedLifecycleEvents())

                .isEmpty();

    }



    @Test

    public void unbind_whileConnected_invokesLifecycleCallbacks() throws Exception {

        connectAndWaitForDone();

        unbindAndWaitForDone();



        assertThat(mCapturingServiceLifecycleCallbacks.getCapturedLifecycleEvents())

                .containsExactly(

                        ServiceLifeCycleEvent.ON_CONNECTED,

                        ServiceLifeCycleEvent.ON_DISCONNECTED)

                .inOrder();

    }





    @Test

    public void unbind_alreadyUnbound_invokesLifecycleCallbacks() throws Exception {

        connectAndWaitForDone();

        unbindAndWaitForDone();

        unbindAndWaitForDone();



        assertThat(mCapturingServiceLifecycleCallbacks.getCapturedLifecycleEvents())

                .containsExactly(

                        ServiceLifeCycleEvent.ON_CONNECTED,

                        ServiceLifeCycleEvent.ON_DISCONNECTED)

                .inOrder();

    }



    @Test

    public void binds_connectsAndUnbindsMultipleTimes_invokesLifecycleCallbacks() throws Exception {

        connectAndWaitForDone();

        unbindAndWaitForDone();

        connectAndWaitForDone();

        unbindAndWaitForDone();

        connectAndWaitForDone();



        assertThat(mCapturingServiceLifecycleCallbacks.getCapturedLifecycleEvents())

                .containsExactly(

                        ServiceLifeCycleEvent.ON_CONNECTED,

                        ServiceLifeCycleEvent.ON_DISCONNECTED,

                        ServiceLifeCycleEvent.ON_CONNECTED,

                        ServiceLifeCycleEvent.ON_DISCONNECTED,

                        ServiceLifeCycleEvent.ON_CONNECTED)

                .inOrder();

    }



    @Test

    public void processCrashes_whileConnected_invokesLifecycleCallbacks() throws Exception {

        connectAndWaitForDone();

        waitForDone(mServiceConnector.post(service -> service.crashProcess()));



        assertThat(mCapturingServiceLifecycleCallbacks.getCapturedLifecycleEvents())

                .containsExactly(

                        ServiceLifeCycleEvent.ON_CONNECTED,

                        ServiceLifeCycleEvent.ON_BINDER_DIED)

                .inOrder();

    }



    private void connectAndWaitForDone() {

        waitForDone(mServiceConnector.connect());

    }



    private void unbindAndWaitForDone() {

        mServiceConnector.unbind();

        InstrumentationRegistry.getInstrumentation().waitForIdleSync();

    }



    private static void waitForDone(AndroidFuture<?> androidFuture) {

        if (androidFuture.isDone()) {

            return;

        }



        try {

            androidFuture.get(10, TimeUnit.SECONDS);

        } catch (InterruptedException | TimeoutException ex) {

            throw new RuntimeException(ex);

        } catch (ExecutionException | CancellationException ex) {

            // Failed and canceled futures are completed

            return;

        }

    }



    public static final class CapturingServiceLifecycleCallbacks implements

            ServiceConnector.ServiceLifecycleCallbacks<ITestServiceConnectorService> {

        public enum ServiceLifeCycleEvent {

            ON_CONNECTED,

            ON_DISCONNECTED,

            ON_BINDER_DIED,

        }



        private final ArrayList<ServiceLifeCycleEvent> mCapturedLifecycleEventServices =

                new ArrayList<>();



        public ArrayList<ServiceLifeCycleEvent> getCapturedLifecycleEvents() {

            return mCapturedLifecycleEventServices;

        }



        @Override

        public void onConnected(@NonNull ITestServiceConnectorService service) {

            requireNonNull(service);

            mCapturedLifecycleEventServices.add(ServiceLifeCycleEvent.ON_CONNECTED);

        }



        @Override

        public void onDisconnected(@NonNull ITestServiceConnectorService service) {

            requireNonNull(service);

            mCapturedLifecycleEventServices.add(ServiceLifeCycleEvent.ON_DISCONNECTED);

        }



        @Override

        public void onBinderDied() {

            mCapturedLifecycleEventServices.add(ServiceLifeCycleEvent.ON_BINDER_DIED);

        }

    }



    public static final class TestService extends Service {



        public static String ACTION_TEST_SERVICE = "android.intent.action.BIND_TEST_SERVICE";



        @Nullable

        @Override

        public IBinder onBind(@Nullable Intent intent) {

            if (intent == null) {

                return null;

            }



            if (!intent.getAction().equals(ACTION_TEST_SERVICE)) {

                return null;

            }



            return new TestServiceConnectorService().asBinder();

        }

    }



    private static final class TestServiceConnectorService extends

            ITestServiceConnectorService.Stub {

        @Override

        public void crashProcess() {

            Process.killProcess(Process.myPid());

        }

    }

}

services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java

+55 −19
Original line number Diff line number Diff line
@@ -50,6 +50,7 @@ import com.android.internal.annotations.GuardedBy; 
import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.infra.AndroidFuture;

import com.android.internal.infra.ServiceConnector;

import com.android.internal.infra.ServiceConnector.ServiceLifecycleCallbacks;

import com.android.server.wm.WindowManagerInternal;

import com.android.server.wm.WindowManagerInternal.TaskSystemBarsListener;

import com.android.server.wm.WindowManagerService;
@@ -64,6 +65,40 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan 
    private static final int CREATE_GAME_SESSION_TIMEOUT_MS = 10_000;

    private static final boolean DEBUG = false;



    private final ServiceLifecycleCallbacks<IGameService> mGameServiceLifecycleCallbacks =

            new ServiceLifecycleCallbacks<IGameService>() {

                @Override

                public void onConnected(@NonNull IGameService service) {

                    try {

                        service.connected(mGameServiceController);

                    } catch (RemoteException ex) {

                        Slog.w(TAG, "Failed to send connected event", ex);

                    }

                }



                @Override

                public void onDisconnected(@NonNull IGameService service) {

                    try {

                        service.disconnected();

                    } catch (RemoteException ex) {

                        Slog.w(TAG, "Failed to send disconnected event", ex);

                    }

                }

            };



    private final ServiceLifecycleCallbacks<IGameSessionService>

            mGameSessionServiceLifecycleCallbacks =

            new ServiceLifecycleCallbacks<IGameSessionService>() {

                @Override

                public void onBinderDied() {

                    mBackgroundExecutor.execute(() -> {

                        synchronized (mLock) {

                            destroyAndClearAllGameSessionsLocked();

                        }

                    });

                }

            };



    private final TaskSystemBarsListener mTaskSystemBarsVisibilityListener =

            new TaskSystemBarsListener() {

                @Override
@@ -206,11 +241,11 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan 
        }

        mIsRunning = true;



        // TODO(b/204503192): In cases where the connection to the game service fails retry with

        //  back off mechanism.

        AndroidFuture<Void> unusedPostConnectedFuture = mGameServiceConnector.post(gameService -> {

            gameService.connected(mGameServiceController);

        });

        mGameServiceConnector.setServiceLifecycleCallbacks(mGameServiceLifecycleCallbacks);

        mGameSessionServiceConnector.setServiceLifecycleCallbacks(

                mGameSessionServiceLifecycleCallbacks);



        AndroidFuture<?> unusedConnectFuture = mGameServiceConnector.connect();



        try {

            mActivityTaskManager.registerTaskStackListener(mTaskStackListener);
@@ -237,19 +272,14 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan 
        mWindowManagerInternal.unregisterTaskSystemBarsListener(

                mTaskSystemBarsVisibilityListener);



        for (GameSessionRecord gameSessionRecord : mGameSessions.values()) {

            destroyGameSessionFromRecordLocked(gameSessionRecord);

        }

        mGameSessions.clear();

        destroyAndClearAllGameSessionsLocked();



        // TODO(b/204503192): It is possible that the game service is disconnected. In this

        //  case we should avoid rebinding just to shut it down again.

        mGameServiceConnector.post(gameService -> {

            gameService.disconnected();

        }).whenComplete((result, t) -> {

        mGameServiceConnector.unbind();

        });

        mGameSessionServiceConnector.unbind();



        mGameServiceConnector.setServiceLifecycleCallbacks(null);

        mGameSessionServiceConnector.setServiceLifecycleCallbacks(null);



    }



    private void onTaskCreated(int taskId, @NonNull ComponentName componentName) {
@@ -488,6 +518,14 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan 
                        createGameSessionResult.getSurfacePackage()));

    }



    @GuardedBy("mLock")

    private void destroyAndClearAllGameSessionsLocked() {

        for (GameSessionRecord gameSessionRecord : mGameSessions.values()) {

            destroyGameSessionFromRecordLocked(gameSessionRecord);

        }

        mGameSessions.clear();

    }



    private void destroyGameSessionDuringAttach(

            int taskId,

            CreateGameSessionResult createGameSessionResult) {
@@ -544,11 +582,9 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan 
                Slog.d(TAG, "No active game sessions. Disconnecting GameSessionService");

            }



            if (mGameSessionServiceConnector != null) {

            mGameSessionServiceConnector.unbind();

        }

    }

    }



    @Nullable

    private GameSessionViewHostConfiguration createViewHostConfigurationForTask(int taskId) {