Implement onBindingDied callback to catch client crashes.
This CL makes the following changes in binding behavior:
- onBindingDied is implemented to schedule a rebind
- this resolves the case where a client crashes before the connection
is established
- rebind is split into scheduleRebind and doRebind operations
- scheduleRebind will only schedule a task if there isn't one pending
- doRebind only rebinds if the service did not reconnect on its own
- unbinding the ServiceConnection is deferred until doRebind
The resulting behavior is such that the first few crashes of a client will
be re-bound faster (as ActivityManager will restart it after 2, 4, and 8
seconds), but after the configured delay the client manager will then
unbind + bind, retaining the existing behavior of capping the backoff.
Bug: 137656857
Test: Manual testing against P.car and Q
Test: Updated VmsClientManagerTest
Test: atest AndroidCarApiTest CarServiceTest CarServiceUnitTest
Change-Id: I1372eae1b0b7fcbe77f982e59236fff71df13356
(cherry picked from commit 8d64eb3f5fe522ac7ced070b789c9c0ced79a8f7)
diff --git a/service/src/com/android/car/vms/VmsClientManager.java b/service/src/com/android/car/vms/VmsClientManager.java
index 24f66b9..ba92a46 100644
--- a/service/src/com/android/car/vms/VmsClientManager.java
+++ b/service/src/com/android/car/vms/VmsClientManager.java
@@ -359,6 +359,7 @@
private final String mFullName;
private boolean mIsBound = false;
private boolean mIsTerminated = false;
+ private boolean mRebindScheduled = false;
private IBinder mClientService;
ClientConnection(ComponentName name, UserHandle user) {
@@ -400,33 +401,54 @@
Log.e(TAG, "While unbinding " + mFullName, t);
}
mIsBound = false;
- if (mClientService != null) {
- notifyListenersOnClientDisconnected(mFullName);
- }
- mClientService = null;
}
- synchronized void rebind() {
- unbind();
+ synchronized void scheduleRebind() {
+ if (mRebindScheduled) {
+ return;
+ }
+
if (DBG) {
Log.d(TAG,
String.format("rebinding %s after %dms", mFullName, mMillisBeforeRebind));
}
- if (!mIsTerminated) {
- mHandler.postDelayed(this::bind, mMillisBeforeRebind);
- synchronized (mRebindCounts) {
- mRebindCounts.computeIfAbsent(mName.getPackageName(), k -> new AtomicLong())
- .incrementAndGet();
- }
+ mHandler.postDelayed(this::doRebind, mMillisBeforeRebind);
+ mRebindScheduled = true;
+ }
+
+ synchronized void doRebind() {
+ mRebindScheduled = false;
+ // Do not rebind if the connection has been terminated, or the client service has
+ // reconnected on its own.
+ if (mIsTerminated || mClientService != null) {
+ return;
+ }
+
+ if (DBG) Log.d(TAG, "rebinding: " + mFullName);
+ // Ensure that the client is not bound before attempting to rebind.
+ // If the client is not currently bound, unbind() will have no effect.
+ unbind();
+ bind();
+ synchronized (mRebindCounts) {
+ mRebindCounts.computeIfAbsent(mName.getPackageName(), k -> new AtomicLong())
+ .incrementAndGet();
}
}
synchronized void terminate() {
if (DBG) Log.d(TAG, "terminating: " + mFullName);
mIsTerminated = true;
+ notifyOnDisconnect();
unbind();
}
+ synchronized void notifyOnDisconnect() {
+ if (mClientService != null) {
+ notifyListenersOnClientDisconnected(mFullName);
+ mClientService = null;
+ }
+ }
+
synchronized void notifyIfConnected(ConnectionListener listener) {
if (mClientService != null) {
listener.onClientConnected(mFullName, mClientService);
@@ -443,7 +465,15 @@
@Override
public void onServiceDisconnected(ComponentName name) {
if (DBG) Log.d(TAG, "onServiceDisconnected: " + mFullName);
- rebind();
+ notifyOnDisconnect();
+ scheduleRebind();
+ }
+
+ @Override
+ public void onBindingDied(ComponentName name) {
+ if (DBG) Log.d(TAG, "onBindingDied: " + mFullName);
+ notifyOnDisconnect();
+ scheduleRebind();
}
@Override
diff --git a/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java b/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java
index f21413e..77d0847 100644
--- a/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java
+++ b/tests/carservice_unit_test/src/com/android/car/vms/VmsClientManagerTest.java
@@ -487,26 +487,63 @@
connection.onServiceConnected(null, new Binder());
connection.onServiceDisconnected(null);
- verify(mContext).unbindService(connection);
verify(mConnectionListener).onClientDisconnected(eq(SYSTEM_CLIENT_NAME));
Thread.sleep(10);
+ verify(mContext).unbindService(connection);
verifySystemBind(1);
}
@Test
- public void testOnSystemServiceDisconnected_ServiceNotConnected() throws Exception {
+ public void testOnSystemServiceDisconnected_ServiceReboundByAndroid() throws Exception {
notifySystemUserUnlocked();
verifySystemBind(1);
resetContext();
ServiceConnection connection = mConnectionCaptor.getValue();
+ connection.onServiceConnected(null, new Binder());
connection.onServiceDisconnected(null);
+ verify(mConnectionListener).onClientDisconnected(eq(SYSTEM_CLIENT_NAME));
+
+ IBinder binder = new Binder();
+ connection.onServiceConnected(null, binder);
+ verify(mConnectionListener).onClientConnected(eq(SYSTEM_CLIENT_NAME), eq(binder));
+ // No more interactions (verified by tearDown)
+ }
+
+
+ @Test
+ public void testOnSystemServiceBindingDied() throws Exception {
+ notifySystemUserUnlocked();
+ verifySystemBind(1);
+ resetContext();
+
+ ServiceConnection connection = mConnectionCaptor.getValue();
+ connection.onServiceConnected(null, new Binder());
+ connection.onServiceDisconnected(null);
+ connection.onBindingDied(null);
+
+ verify(mConnectionListener).onClientDisconnected(eq(SYSTEM_CLIENT_NAME));
+
+ Thread.sleep(10);
verify(mContext).unbindService(connection);
+ verifySystemBind(1);
+ }
+
+ @Test
+ public void testOnSystemServiceBindingDied_ServiceNotConnected() throws Exception {
+ notifySystemUserUnlocked();
+ verifySystemBind(1);
+ resetContext();
+
+ ServiceConnection connection = mConnectionCaptor.getValue();
+ connection.onBindingDied(null);
+
verifyZeroInteractions(mConnectionListener);
Thread.sleep(10);
+ verify(mContext).unbindService(connection);
verifySystemBind(1);
}
@@ -520,26 +557,62 @@
connection.onServiceConnected(null, new Binder());
connection.onServiceDisconnected(null);
- verify(mContext).unbindService(connection);
verify(mConnectionListener).onClientDisconnected(eq(USER_CLIENT_NAME));
Thread.sleep(10);
+ verify(mContext).unbindService(connection);
verifyUserBind(1);
}
@Test
- public void testOnUserServiceDisconnected_ServiceNotConnected() throws Exception {
+ public void testOnUserServiceDisconnected_ServiceReboundByAndroid() throws Exception {
notifyUserUnlocked();
verifyUserBind(1);
resetContext();
ServiceConnection connection = mConnectionCaptor.getValue();
+ connection.onServiceConnected(null, new Binder());
connection.onServiceDisconnected(null);
+ verify(mConnectionListener).onClientDisconnected(eq(USER_CLIENT_NAME));
+
+ IBinder binder = new Binder();
+ connection.onServiceConnected(null, binder);
+ verify(mConnectionListener).onClientConnected(eq(USER_CLIENT_NAME), eq(binder));
+ // No more interactions (verified by tearDown)
+ }
+
+ @Test
+ public void testOnUserServiceBindingDied() throws Exception {
+ notifyUserUnlocked();
+ verifyUserBind(1);
+ resetContext();
+
+ ServiceConnection connection = mConnectionCaptor.getValue();
+ connection.onServiceConnected(null, new Binder());
+ connection.onServiceDisconnected(null);
+ connection.onBindingDied(null);
+
+ verify(mConnectionListener).onClientDisconnected(eq(USER_CLIENT_NAME));
+
+ Thread.sleep(10);
verify(mContext).unbindService(connection);
+ verifyUserBind(1);
+ }
+
+ @Test
+ public void testOnUserServiceBindingDied_ServiceNotConnected() throws Exception {
+ notifyUserUnlocked();
+ verifyUserBind(1);
+ resetContext();
+
+ ServiceConnection connection = mConnectionCaptor.getValue();
+ connection.onBindingDied(null);
+
verifyZeroInteractions(mConnectionListener);
Thread.sleep(10);
+ verify(mContext).unbindService(connection);
verifyUserBind(1);
}