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

Commit 93f1ed2a authored by Joël Stemmer's avatar Joël Stemmer
Browse files

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
parent 55f19502

services/backup/java/com/android/server/backup/transport/TransportConnection.java

+11 −3
Original line number Diff line number Diff line
@@ -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) {

            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,

services/robotests/backup/src/com/android/server/backup/transport/TransportConnectionTest.java

+13 −0
Original line number Diff line number Diff line
@@ -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);