From e9d38721dbd3fc2e50ff7818555ccd2e9edcef2f Mon Sep 17 00:00:00 2001 From: Nathan Harold Date: Fri, 15 May 2020 15:25:47 -0700 Subject: Abort NetworkScans when Phone Process Crashes If the phone process crashes during a network scan, today the scan hangs indefinitely. This CL adds a binder death recipient to the wakefulness binder so that if the phone process crashes, we detect it and cancel the scan. Because there's no error code for "Telephony crashed" today, the closest error code is to say that the modem is unavailable. In addition, fix an issue where onError() did not actually remove scans from the list of cached scans. This left dangling scan objects in the cache after an error. Bug: 155853346 Test: manual - 1) start scan 2) crash phone process 3) verify error is returned to scan request Change-Id: I8bd3823805fcc68623a685848517f2d11555e9c7 --- .../android/telephony/TelephonyScanManager.java | 84 +++++++++++++++++----- 1 file changed, 67 insertions(+), 17 deletions(-) diff --git a/telephony/java/android/telephony/TelephonyScanManager.java b/telephony/java/android/telephony/TelephonyScanManager.java index 359d08d294c3..3f8fd9d68a4f 100644 --- a/telephony/java/android/telephony/TelephonyScanManager.java +++ b/telephony/java/android/telephony/TelephonyScanManager.java @@ -31,6 +31,7 @@ import android.os.RemoteException; import android.os.ServiceManager; import android.util.SparseArray; +import com.android.internal.annotations.GuardedBy; import com.android.internal.telephony.ITelephony; import com.android.telephony.Rlog; @@ -56,6 +57,8 @@ public final class TelephonyScanManager { public static final int CALLBACK_SCAN_COMPLETE = 3; /** @hide */ public static final int CALLBACK_RESTRICTED_SCAN_RESULTS = 4; + /** @hide */ + public static final int CALLBACK_TELEPHONY_DIED = 5; /** @hide */ public static final int INVALID_SCAN_ID = -1; @@ -104,17 +107,44 @@ public final class TelephonyScanManager { } private final Looper mLooper; + private final Handler mHandler; private final Messenger mMessenger; private final SparseArray mScanInfo = new SparseArray(); + private final Binder.DeathRecipient mDeathRecipient; public TelephonyScanManager() { HandlerThread thread = new HandlerThread(TAG); thread.start(); mLooper = thread.getLooper(); - mMessenger = new Messenger(new Handler(mLooper) { + mHandler = new Handler(mLooper) { @Override public void handleMessage(Message message) { checkNotNull(message, "message cannot be null"); + if (message.what == CALLBACK_TELEPHONY_DIED) { + // If there are no objects in mScanInfo then binder death will simply return. + synchronized (mScanInfo) { + for (int i = 0; i < mScanInfo.size(); i++) { + NetworkScanInfo nsi = mScanInfo.valueAt(i); + // At this point we go into panic mode and ignore errors that would + // normally stop the show in order to try and clean up as gracefully + // as possible. + if (nsi == null) continue; // shouldn't be possible + Executor e = nsi.mExecutor; + NetworkScanCallback cb = nsi.mCallback; + if (e == null || cb == null) continue; + try { + e.execute( + () -> cb.onError(NetworkScan.ERROR_MODEM_UNAVAILABLE)); + } catch (java.util.concurrent.RejectedExecutionException ignore) { + // ignore so that we can continue + } + } + + mScanInfo.clear(); + } + return; + } + NetworkScanInfo nsi; synchronized (mScanInfo) { nsi = mScanInfo.get(message.arg2); @@ -159,6 +189,9 @@ public final class TelephonyScanManager { Rlog.d(TAG, "onError: " + errorCode); callback.onError(errorCode); }); + synchronized (mScanInfo) { + mScanInfo.remove(message.arg2); + } } catch (Exception e) { Rlog.e(TAG, "Exception in networkscan callback onError", e); } @@ -169,7 +202,9 @@ public final class TelephonyScanManager { Rlog.d(TAG, "onComplete"); callback.onComplete(); }); - mScanInfo.remove(message.arg2); + synchronized (mScanInfo) { + mScanInfo.remove(message.arg2); + } } catch (Exception e) { Rlog.e(TAG, "Exception in networkscan callback onComplete", e); } @@ -179,7 +214,14 @@ public final class TelephonyScanManager { break; } } - }); + }; + mMessenger = new Messenger(mHandler); + mDeathRecipient = new Binder.DeathRecipient() { + @Override + public void binderDied() { + mHandler.obtainMessage(CALLBACK_TELEPHONY_DIED).sendToTarget(); + } + }; } /** @@ -190,7 +232,7 @@ public final class TelephonyScanManager { * *

* Requires Permission: - * {@link android.Manifest.permission#ACCESS_COARSE_LOCATION} and + * {@link android.Manifest.permission#ACCESS_FINE_LOCATION} and * {@link android.Manifest.permission#MODIFY_PHONE_STATE MODIFY_PHONE_STATE} * Or the calling app has carrier privileges. @see #hasCarrierPrivileges * @@ -203,19 +245,26 @@ public final class TelephonyScanManager { NetworkScanRequest request, Executor executor, NetworkScanCallback callback, String callingPackage, String callingFeatureId) { try { - ITelephony telephony = getITelephony(); - if (telephony != null) { - 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; - } - saveScanInfo(scanId, request, executor, callback); - return new NetworkScan(scanId, subId); - } + 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; + } + 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: + // - Duplicate links to death with the same callback do not result in + // extraneous callbacks (the tracking de-dupes). + // - Receiving binderDeath() when no scans are active is a no-op. + telephony.asBinder().linkToDeath(mDeathRecipient, 0); + saveScanInfo(scanId, request, executor, callback); + return new NetworkScan(scanId, subId); } } catch (RemoteException ex) { Rlog.e(TAG, "requestNetworkScan RemoteException", ex); @@ -225,6 +274,7 @@ public final class TelephonyScanManager { return null; } + @GuardedBy("mScanInfo") private void saveScanInfo( int id, NetworkScanRequest request, Executor executor, NetworkScanCallback callback) { mScanInfo.put(id, new NetworkScanInfo(request, executor, callback)); -- cgit v1.2.3-59-g8ed1b