diff options
7 files changed, 455 insertions, 24 deletions
diff --git a/core/java/com/android/internal/infra/ServiceConnector.java b/core/java/com/android/internal/infra/ServiceConnector.java index 9ced6097804d..9e07f973b3c4 100644 --- a/core/java/com/android/internal/infra/ServiceConnector.java +++ b/core/java/com/android/internal/infra/ServiceConnector.java @@ -147,6 +147,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. + } } } diff --git a/core/tests/coretests/AndroidManifest.xml b/core/tests/coretests/AndroidManifest.xml index a80424e500c4..04857ec756d2 100644 --- a/core/tests/coretests/AndroidManifest.xml +++ b/core/tests/coretests/AndroidManifest.xml @@ -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" /> diff --git a/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/ITestServiceConnectorService.aidl b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/ITestServiceConnectorService.aidl new file mode 100644 index 000000000000..09544b3522b0 --- /dev/null +++ b/core/tests/coretests/aidl/com/android/frameworks/coretests/aidl/ITestServiceConnectorService.aidl @@ -0,0 +1,21 @@ +/* + * 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(); +} diff --git a/core/tests/coretests/src/com/android/internal/infra/ServiceConnectorTest.java b/core/tests/coretests/src/com/android/internal/infra/ServiceConnectorTest.java new file mode 100644 index 000000000000..725dcf30d485 --- /dev/null +++ b/core/tests/coretests/src/com/android/internal/infra/ServiceConnectorTest.java @@ -0,0 +1,242 @@ +/* + * 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()); + } + } +} + diff --git a/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java b/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java index e8d9dadcb17a..e4edf4e0f5f9 100644 --- a/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java +++ b/services/core/java/com/android/server/app/GameServiceProviderInstanceImpl.java @@ -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(); - }); + 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,9 +582,7 @@ final class GameServiceProviderInstanceImpl implements GameServiceProviderInstan Slog.d(TAG, "No active game sessions. Disconnecting GameSessionService"); } - if (mGameSessionServiceConnector != null) { - mGameSessionServiceConnector.unbind(); - } + mGameSessionServiceConnector.unbind(); } } diff --git a/services/tests/mockingservicestests/src/com/android/server/app/FakeServiceConnector.java b/services/tests/mockingservicestests/src/com/android/server/app/FakeServiceConnector.java index 0ae509ec0fbc..182430d1a358 100644 --- a/services/tests/mockingservicestests/src/com/android/server/app/FakeServiceConnector.java +++ b/services/tests/mockingservicestests/src/com/android/server/app/FakeServiceConnector.java @@ -17,6 +17,7 @@ package com.android.server.app; +import android.annotation.Nullable; import android.os.IInterface; import com.android.internal.infra.AndroidFuture; @@ -30,9 +31,10 @@ import java.util.concurrent.CompletableFuture; * Tests provide a service instance via {@link #FakeServiceConnector(IInterface)} that will be * connected to and used to fulfill service jobs. */ -final class FakeServiceConnector<T extends IInterface> implements - ServiceConnector<T> { +final class FakeServiceConnector<T extends IInterface> implements ServiceConnector<T> { private final T mService; + @Nullable + private ServiceLifecycleCallbacks mServiceLifecycleCallbacks; private boolean mIsConnected; private int mConnectCount = 0; @@ -96,9 +98,17 @@ final class FakeServiceConnector<T extends IInterface> implements @Override public void unbind() { + if (mServiceLifecycleCallbacks != null) { + mServiceLifecycleCallbacks.onDisconnected(mService); + } mIsConnected = false; } + @Override + public void setServiceLifecycleCallbacks(@Nullable ServiceLifecycleCallbacks<T> callbacks) { + mServiceLifecycleCallbacks = callbacks; + } + private void markPossibleConnection() { if (mIsConnected) { return; @@ -106,6 +116,21 @@ final class FakeServiceConnector<T extends IInterface> implements mConnectCount += 1; mIsConnected = true; + + if (mServiceLifecycleCallbacks != null) { + mServiceLifecycleCallbacks.onConnected(mService); + } + } + + public void killServiceProcess() { + if (!mIsConnected) { + return; + } + mIsConnected = false; + + if (mServiceLifecycleCallbacks != null) { + mServiceLifecycleCallbacks.onBinderDied(); + } } /** diff --git a/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java b/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java index ed232e5458b0..32a31d0e57f7 100644 --- a/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java +++ b/services/tests/mockingservicestests/src/com/android/server/app/GameServiceProviderInstanceImplTest.java @@ -713,6 +713,35 @@ public final class GameServiceProviderInstanceImplTest { } @Test + public void gameSessionServiceDies_severalActiveGameSessions_destroysGameSessions() { + mGameServiceProviderInstance.start(); + + startTask(10, GAME_A_MAIN_ACTIVITY); + mockPermissionGranted(Manifest.permission.MANAGE_GAME_ACTIVITY); + mFakeGameService.requestCreateGameSession(10); + + FakeGameSession gameSession10 = new FakeGameSession(); + SurfacePackage mockSurfacePackage10 = Mockito.mock(SurfacePackage.class); + mFakeGameSessionService.removePendingFutureForTaskId(10) + .complete(new CreateGameSessionResult(gameSession10, mockSurfacePackage10)); + + startTask(11, GAME_A_MAIN_ACTIVITY); + mFakeGameService.requestCreateGameSession(11); + + FakeGameSession gameSession11 = new FakeGameSession(); + SurfacePackage mockSurfacePackage11 = Mockito.mock(SurfacePackage.class); + mFakeGameSessionService.removePendingFutureForTaskId(11) + .complete(new CreateGameSessionResult(gameSession11, mockSurfacePackage11)); + + mFakeGameSessionServiceConnector.killServiceProcess(); + + assertThat(gameSession10.mIsDestroyed).isTrue(); + assertThat(gameSession11.mIsDestroyed).isTrue(); + assertThat(mFakeGameServiceConnector.getIsConnected()).isTrue(); + assertThat(mFakeGameSessionServiceConnector.getIsConnected()).isFalse(); + } + + @Test public void stop_severalActiveGameSessions_destroysGameSessionsAndUnbinds() throws Exception { mGameServiceProviderInstance.start(); |