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

Commit edbe38f8 authored by Kyunglyul Hyun's avatar Kyunglyul Hyun
Browse files

Unregister scan client when RemoteException raises 

When a remote exception is thrown while delivering
scan results, stop scanning and unregister the client.

As we marked "appDied=true", the client will be
unregistered while stop scanning.

Bug: 320223821
Bug: 320402249
Test: atest GattServiceTest ScanManagerTest &&
 manually crash apps multiple times to check

Change-Id: I570ed270751ad0cb47a8da37154873536ab24b3c
parent 4e209562

android/app/src/com/android/bluetooth/gatt/GattService.java

+28 −6
Original line number Original line Diff line number Diff line
@@ -94,6 +94,7 @@ import com.android.bluetooth.btservice.MetricsLogger; 
import com.android.bluetooth.btservice.ProfileService;

import com.android.bluetooth.btservice.ProfileService;

import com.android.bluetooth.flags.FeatureFlags;

import com.android.bluetooth.flags.FeatureFlags;

import com.android.bluetooth.flags.FeatureFlagsImpl;

import com.android.bluetooth.flags.FeatureFlagsImpl;

import com.android.bluetooth.flags.Flags;

import com.android.bluetooth.util.NumberUtils;

import com.android.bluetooth.util.NumberUtils;

import com.android.internal.annotations.VisibleForTesting;

import com.android.internal.annotations.VisibleForTesting;

import com.android.modules.utils.SynchronousResultReceiver;

import com.android.modules.utils.SynchronousResultReceiver;
@@ -533,10 +534,14 @@ public class GattService extends ProfileService { 




            ScanClient client = getScanClient(mScannerId);

            ScanClient client = getScanClient(mScannerId);

            if (client != null) {

            if (client != null) {

                if (Flags.leScanFixRemoteException()) {

                    handleDeadScanClient(client);

                } else {

                    client.appDied = true;

                    client.appDied = true;

                    stopScan(client.scannerId, getAttributionSource());

                    stopScan(client.scannerId, getAttributionSource());

                }

                }

            }

            }

        }





        private ScanClient getScanClient(int clientIf) {

        private ScanClient getScanClient(int clientIf) {

            for (ScanClient client : mScanManager.getRegularScanQueue()) {

            for (ScanClient client : mScanManager.getRegularScanQueue()) {
@@ -2038,11 +2043,15 @@ public class GattService extends ProfileService { 
                }

                }

            } catch (RemoteException | PendingIntent.CanceledException e) {

            } catch (RemoteException | PendingIntent.CanceledException e) {

                Log.e(TAG, "Exception: " + e);

                Log.e(TAG, "Exception: " + e);

                if (Flags.leScanFixRemoteException()) {

                    handleDeadScanClient(client);

                } else {

                    mScannerMap.remove(client.scannerId);

                    mScannerMap.remove(client.scannerId);

                    mScanManager.stopScan(client.scannerId);

                    mScanManager.stopScan(client.scannerId);

                }

                }

            }

            }

        }

        }

    }





    private void sendResultByPendingIntent(PendingIntentInfo pii, ScanResult result,

    private void sendResultByPendingIntent(PendingIntentInfo pii, ScanResult result,

            int callbackType, ScanClient client) {

            int callbackType, ScanClient client) {
@@ -2144,6 +2153,15 @@ public class GattService extends ProfileService { 
        return new MatchResult(false, MatchOrigin.PSEUDO_ADDRESS);

        return new MatchResult(false, MatchOrigin.PSEUDO_ADDRESS);

    }

    }





    private void handleDeadScanClient(ScanClient client) {

        if (client.appDied) {

            Log.w(TAG, "Already dead client " + client.scannerId);

            return;

        }

        client.appDied = true;

        stopScan(client.scannerId, getAttributionSource());

    }



    void onClientRegistered(int status, int clientIf, long uuidLsb, long uuidMsb)

    void onClientRegistered(int status, int clientIf, long uuidLsb, long uuidMsb)

            throws RemoteException {

            throws RemoteException {

        UUID uuid = new UUID(uuidMsb, uuidLsb);

        UUID uuid = new UUID(uuidMsb, uuidLsb);
@@ -2789,10 +2807,14 @@ public class GattService extends ProfileService { 
            }

            }

        } catch (RemoteException | PendingIntent.CanceledException e) {

        } catch (RemoteException | PendingIntent.CanceledException e) {

            Log.e(TAG, "Exception: " + e);

            Log.e(TAG, "Exception: " + e);

            if (Flags.leScanFixRemoteException()) {

                handleDeadScanClient(client);

            } else {

                mScannerMap.remove(client.scannerId);

                mScannerMap.remove(client.scannerId);

                mScanManager.stopScan(client.scannerId);

                mScanManager.stopScan(client.scannerId);

            }

            }

        }

        }

    }





    // Check and deliver scan results for different scan clients.

    // Check and deliver scan results for different scan clients.

    private void deliverBatchScan(ScanClient client, Set<ScanResult> allResults)

    private void deliverBatchScan(ScanClient client, Set<ScanResult> allResults)

android/app/tests/unit/src/com/android/bluetooth/gatt/GattServiceTest.java

+62 −0
Original line number Original line Diff line number Diff line
@@ -46,6 +46,7 @@ import android.os.Binder; 
import android.os.RemoteException;

import android.os.RemoteException;

import android.os.WorkSource;

import android.os.WorkSource;





import android.platform.test.annotations.RequiresFlagsEnabled;

import androidx.test.InstrumentationRegistry;

import androidx.test.InstrumentationRegistry;

import androidx.test.filters.SmallTest;

import androidx.test.filters.SmallTest;

import androidx.test.rule.ServiceTestRule;

import androidx.test.rule.ServiceTestRule;
@@ -55,8 +56,10 @@ import com.android.bluetooth.TestUtils; 
import com.android.bluetooth.btservice.AdapterService;

import com.android.bluetooth.btservice.AdapterService;

import com.android.bluetooth.btservice.CompanionManager;

import com.android.bluetooth.btservice.CompanionManager;





import com.android.bluetooth.flags.Flags;

import org.junit.After;

import org.junit.After;

import org.junit.Assert;

import org.junit.Assert;

import org.junit.Assume;

import org.junit.Before;

import org.junit.Before;

import org.junit.Rule;

import org.junit.Rule;

import org.junit.Test;

import org.junit.Test;
@@ -66,6 +69,7 @@ import org.mockito.Mock; 
import org.mockito.MockitoAnnotations;

import org.mockito.MockitoAnnotations;





import java.util.ArrayList;

import java.util.ArrayList;

import java.util.Collections;

import java.util.HashMap;

import java.util.HashMap;

import java.util.HashSet;

import java.util.HashSet;

import java.util.List;

import java.util.List;
@@ -487,6 +491,64 @@ public class GattServiceTest { 
        verify(mScanManager).flushBatchScanResults(new ScanClient(scannerId));

        verify(mScanManager).flushBatchScanResults(new ScanClient(scannerId));

    }

    }





    @RequiresFlagsEnabled(Flags.FLAG_LE_SCAN_FIX_REMOTE_EXCEPTION)

    @Test

    public void onScanResult_remoteException_clientDied() throws Exception {

        Assume.assumeTrue(Flags.leScanFixRemoteException());

        int scannerId = 1;



        int eventType = 0;

        int addressType = 0;

        String address = "02:00:00:00:00:00";

        int primaryPhy = 0;

        int secondPhy = 0;

        int advertisingSid = 0;

        int txPower = 0;

        int rssi = 0;

        int periodicAdvInt = 0;

        byte[] advData = new byte[0];



        ScanClient scanClient = new ScanClient(scannerId);

        scanClient.scannerId = scannerId;

        scanClient.hasNetworkSettingsPermission = true;

        scanClient.settings =

                new ScanSettings.Builder()

                        .setCallbackType(ScanSettings.CALLBACK_TYPE_ALL_MATCHES)

                        .setLegacy(false)

                        .build();



        AppScanStats appScanStats = mock(AppScanStats.class);

        IScannerCallback callback = mock(IScannerCallback.class);



        mApp.callback = callback;

        mApp.appScanStats = appScanStats;

        Set<ScanClient> scanClientSet = Collections.singleton(scanClient);



        doReturn(address).when(mAdapterService).getIdentityAddress(anyString());

        doReturn(scanClientSet).when(mScanManager).getRegularScanQueue();

        doReturn(mApp).when(mScannerMap).getById(scanClient.scannerId);

        doReturn(appScanStats).when(mScannerMap).getAppScanStatsById(scanClient.scannerId);



        // Simulate remote client crash

        doThrow(new RemoteException()).when(callback).onScanResult(any());



        mService.onScanResult(

                eventType,

                addressType,

                address,

                primaryPhy,

                secondPhy,

                advertisingSid,

                txPower,

                rssi,

                periodicAdvInt,

                advData,

                address);



        assertThat(scanClient.appDied).isTrue();

        verify(appScanStats).recordScanStop(scannerId);

    }



    @Test

    @Test

    public void readCharacteristic() {

    public void readCharacteristic() {

        int clientIf = 1;

        int clientIf = 1;