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

Commit 1fe18a79 authored by Nathan Harold's avatar Nathan Harold
Browse files

Ensure that ScanInfo is cached before callbacks fire 

Fixes a regression that was introduced in aosp/1312940.

There is a critical section in requestNetworkScan after the scanId
has been generated and before the ScanInfo is added to the cache.
If during this time a callback fires on a separate thread, it will
not yet find the ScanInfo record, and due to strict checking, this
causes a RuntimeException. This CL re-expands the synchronized block
to cover the critical section, and clarifies a little bounds checking.

Note: this change is not ideal. This means that all callbacks for all
scans will be blocked while a request for a new scan is being made,
which was the motivation behind the change that introduced this
regression. But, a narrow performance hit is better than a crash, and
it's not worth the complexity of a bigger fix.

Bug: 200634560
Test: compilation
Change-Id: I4670da109256170121ceb6d8fbad0efda310335f
parent c12df4f3

telephony/java/android/telephony/TelephonyScanManager.java

+18 −17
Original line number Diff line number Diff line
@@ -36,6 +36,7 @@ import com.android.telephony.Rlog; 


import java.util.Arrays;

import java.util.List;

import java.util.Objects;

import java.util.concurrent.Executor;



/**
@@ -152,16 +153,9 @@ public final class TelephonyScanManager { 
                    throw new RuntimeException(

                        "Failed to find NetworkScanInfo with id " + message.arg2);

                }

                NetworkScanCallback callback = nsi.mCallback;

                Executor executor = nsi.mExecutor;

                if (callback == null) {

                    throw new RuntimeException(

                        "Failed to find NetworkScanCallback with id " + message.arg2);

                }

                if (executor == null) {

                    throw new RuntimeException(

                        "Failed to find Executor with id " + message.arg2);

                }



                final NetworkScanCallback callback = nsi.mCallback;

                final Executor executor = nsi.mExecutor;



                switch (message.what) {

                    case CALLBACK_RESTRICTED_SCAN_RESULTS:
@@ -246,9 +240,17 @@ public final class TelephonyScanManager { 
            NetworkScanRequest request, Executor executor, NetworkScanCallback callback,

            String callingPackage, @Nullable String callingFeatureId) {

        try {

            Objects.requireNonNull(request, "Request was null");

            Objects.requireNonNull(callback, "Callback was null");

            Objects.requireNonNull(executor, "Executor was null");

            final ITelephony telephony = getITelephony();

            if (telephony == null) return null;



            // The lock must be taken before calling requestNetworkScan because the resulting

            // scanId can be invoked asynchronously on another thread at any time after

            // requestNetworkScan invoked, leaving a critical section between that call and adding

            // the record to the ScanInfo cache.

            synchronized (mScanInfo) {

                int scanId = telephony.requestNetworkScan(

                        subId, request, mMessenger, new Binder(), callingPackage,

                        callingFeatureId);
@@ -256,7 +258,6 @@ public final class TelephonyScanManager { 
                    Rlog.e(TAG, "Failed to initiate network scan");

                    return null;

                }

            synchronized (mScanInfo) {

                // We link to death whenever a scan is started to ensure that we are linked

                // at the point that phone process death might matter.

                // We never unlink because: