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 Merged-In: I4670da109256170121ceb6d8fbad0efda310335f Change-Id: I4670da109256170121ceb6d8fbad0efda310335f
This commit is contained in:
@@ -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,17 +240,24 @@ 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;
|
||||
|
||||
int scanId = telephony.requestNetworkScan(
|
||||
subId, request, mMessenger, new Binder(), callingPackage,
|
||||
callingFeatureId);
|
||||
if (scanId == INVALID_SCAN_ID) {
|
||||
Rlog.e(TAG, "Failed to initiate network scan");
|
||||
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);
|
||||
if (scanId == INVALID_SCAN_ID) {
|
||||
Rlog.e(TAG, "Failed to initiate network scan");
|
||||
return null;
|
||||
}
|
||||
// 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:
|
||||
|
||||
Reference in New Issue
Block a user