diff options
author | 2024-05-14 11:55:36 +0100 | |
---|---|---|
committer | 2024-05-14 11:55:36 +0100 | |
commit | 93f1ed2a0aa1e332a29feb1ea90a788097f85f60 (patch) | |
tree | f1941e3c05e23e8c2aedebd0e2c8a6a2121c0b66 | |
parent | 55f19502fe62a5b76c0236b2c8df69d5debbb7b6 (diff) |
Catch and ignore `IllegalArgumentException` thrown by `unbindService`
When `TransportConnection` handles `onServiceConnected` or
`onBindingDied` it calls `Context.unbindService`. In a very small number
of cases this fails because the connection is not (or no longer?)
registered. When this happens, the system server crashes causing the
device to reboot. We don't really care if the service is not registered
when we try to unbind it, so we just catch and ignore the exception to
avoid crashing the system server.
Bug: 335547110
Test: atest TransportConnectionTest.java
Change-Id: Ic7f34eacd1f621c42a68128ef992043dba6808f1
2 files changed, 24 insertions, 3 deletions
diff --git a/services/backup/java/com/android/server/backup/transport/TransportConnection.java b/services/backup/java/com/android/server/backup/transport/TransportConnection.java index 1009787bebe5..67ebb3e6a1ef 100644 --- a/services/backup/java/com/android/server/backup/transport/TransportConnection.java +++ b/services/backup/java/com/android/server/backup/transport/TransportConnection.java @@ -658,11 +658,13 @@ public class TransportConnection { * This class is a proxy to TransportClient methods that doesn't hold a strong reference to the * TransportClient, allowing it to be GC'ed. If the reference was lost it logs a message. */ - private static class TransportConnectionMonitor implements ServiceConnection { + @VisibleForTesting + static class TransportConnectionMonitor implements ServiceConnection { private final Context mContext; private final WeakReference<TransportConnection> mTransportClientRef; - private TransportConnectionMonitor(Context context, + @VisibleForTesting + TransportConnectionMonitor(Context context, TransportConnection transportConnection) { mContext = context; mTransportClientRef = new WeakReference<>(transportConnection); @@ -704,7 +706,13 @@ public class TransportConnection { /** @see TransportConnection#finalize() */ private void referenceLost(String caller) { - mContext.unbindService(this); + try { + mContext.unbindService(this); + } catch (IllegalArgumentException e) { + TransportUtils.log(Priority.WARN, TAG, + caller + " called but unbindService failed: " + e.getMessage()); + return; + } TransportUtils.log( Priority.INFO, TAG, diff --git a/services/robotests/backup/src/com/android/server/backup/transport/TransportConnectionTest.java b/services/robotests/backup/src/com/android/server/backup/transport/TransportConnectionTest.java index 6a82f1656414..3e87c6fe7be7 100644 --- a/services/robotests/backup/src/com/android/server/backup/transport/TransportConnectionTest.java +++ b/services/robotests/backup/src/com/android/server/backup/transport/TransportConnectionTest.java @@ -28,6 +28,7 @@ import static org.mockito.ArgumentMatchers.anyInt; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.isNull; import static org.mockito.Mockito.doAnswer; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -543,6 +544,18 @@ public class TransportConnectionTest { return future.get(); } + @Test + public void onBindingDied_referenceLost_doesNotThrow() { + TransportConnection.TransportConnectionMonitor transportConnectionMonitor = + new TransportConnection.TransportConnectionMonitor( + mContext, /* transportConnection= */ null); + doThrow(new IllegalArgumentException("Service not registered")).when( + mContext).unbindService(any()); + + // Test no exception is thrown + transportConnectionMonitor.onBindingDied(mTransportComponent); + } + private ServiceConnection verifyBindServiceAsUserAndCaptureServiceConnection(Context context) { ArgumentCaptor<ServiceConnection> connectionCaptor = ArgumentCaptor.forClass(ServiceConnection.class); |