Combine handling AIDL binder death
The change revive ag/22110298, which was reverted due to interaction with mock modem. The CTS issue was that upon unbinding mock modem, the mocked hal version wasn't cleared when attempting to get radio service, so the radio service is falsely considered as disabled, hence future query will always think it's disabled.
Test: atest RIL
Test: atest android/telephony/cts/TelephonyManagerTestOnMockModem.java
Test: trigger modem crash and verify recovery
Test: voice call + data browsing
Fix: 319612362
Change-Id: Id953a3c7ec6c52b5b81f8828a286b2da91bec46b
diff --git a/flags/misc.aconfig b/flags/misc.aconfig
index a491665..1a23279 100644
--- a/flags/misc.aconfig
+++ b/flags/misc.aconfig
@@ -1,6 +1,17 @@
package: "com.android.internal.telephony.flags"
container: "system"
+# OWNER=linggm TARGET=24Q3
+flag {
+ name: "combine_ril_death_handle"
+ namespace: "telephony"
+ description: "Upon radio service death, combine its handling to prevent race condition"
+ bug:"319612362"
+ metadata {
+ purpose: PURPOSE_BUGFIX
+ }
+}
+
# OWNER=tjstuart TARGET=24Q3
flag {
name: "do_not_override_precise_label"
diff --git a/src/java/com/android/internal/telephony/PhoneFactory.java b/src/java/com/android/internal/telephony/PhoneFactory.java
index 803fb19..d9c5c9c 100644
--- a/src/java/com/android/internal/telephony/PhoneFactory.java
+++ b/src/java/com/android/internal/telephony/PhoneFactory.java
@@ -187,7 +187,7 @@
Rlog.i(LOG_TAG, "Network Mode set to " + Integer.toString(networkModes[i]));
sCommandsInterfaces[i] = new RIL(context,
RadioAccessFamily.getRafFromNetworkType(networkModes[i]),
- cdmaSubscription, i);
+ cdmaSubscription, i, featureFlags);
}
if (numPhones > 0) {
@@ -312,7 +312,7 @@
for (int i = prevActiveModemCount; i < activeModemCount; i++) {
sCommandsInterfaces[i] = new RIL(context, RadioAccessFamily.getRafFromNetworkType(
RILConstants.PREFERRED_NETWORK_MODE),
- cdmaSubscription, i);
+ cdmaSubscription, i, sFeatureFlags);
sPhones[i] = createPhone(context, i);
if (context.getPackageManager().hasSystemFeature(
PackageManager.FEATURE_TELEPHONY_IMS)) {
diff --git a/src/java/com/android/internal/telephony/RIL.java b/src/java/com/android/internal/telephony/RIL.java
index 9e64f52..f1c9dc2 100644
--- a/src/java/com/android/internal/telephony/RIL.java
+++ b/src/java/com/android/internal/telephony/RIL.java
@@ -88,6 +88,7 @@
import com.android.internal.telephony.cdma.CdmaInformationRecords;
import com.android.internal.telephony.cdma.CdmaSmsBroadcastConfigInfo;
import com.android.internal.telephony.emergency.EmergencyConstants;
+import com.android.internal.telephony.flags.FeatureFlags;
import com.android.internal.telephony.gsm.SmsBroadcastConfigInfo;
import com.android.internal.telephony.imsphone.ImsCallInfo;
import com.android.internal.telephony.metrics.ModemRestartStats;
@@ -219,6 +220,8 @@
public static final int MAX_SERVICE_IDX = HAL_SERVICE_IMS;
+ @NonNull private final FeatureFlags mFeatureFlags;
+
/**
* An array of sets that records if services are disabled in the HAL for a specific phone ID
* slot to avoid further getService requests for that service. See XXX_SERVICE for the indices.
@@ -382,12 +385,27 @@
case EVENT_AIDL_PROXY_DEAD:
int aidlService = msg.arg1;
long msgCookie = (long) msg.obj;
- riljLog("handleMessage: EVENT_AIDL_PROXY_DEAD cookie = " + msgCookie
- + ", service = " + serviceToString(aidlService) + ", cookie = "
- + mServiceCookies.get(aidlService));
- if (msgCookie == mServiceCookies.get(aidlService).get()) {
- mIsRadioProxyInitialized = false;
- resetProxyAndRequestList(aidlService);
+ if (mFeatureFlags.combineRilDeathHandle()) {
+ if (msgCookie == mServiceCookies.get(aidlService).get()) {
+ riljLog("handleMessage: EVENT_AIDL_PROXY_DEAD cookie = " + msgCookie
+ + ", service = " + serviceToString(aidlService) + ", cookie = "
+ + mServiceCookies.get(aidlService));
+ mIsRadioProxyInitialized = false;
+ resetProxyAndRequestList(aidlService);
+ // Remove duplicate death message to avoid duplicate reset.
+ mRilHandler.removeMessages(EVENT_AIDL_PROXY_DEAD);
+ } else {
+ riljLog("Ignore stale EVENT_AIDL_PROXY_DEAD for service "
+ + serviceToString(aidlService));
+ }
+ } else {
+ riljLog("handleMessage: EVENT_AIDL_PROXY_DEAD cookie = " + msgCookie
+ + ", service = " + serviceToString(aidlService) + ", cookie = "
+ + mServiceCookies.get(aidlService));
+ if (msgCookie == mServiceCookies.get(aidlService).get()) {
+ mIsRadioProxyInitialized = false;
+ resetProxyAndRequestList(aidlService);
+ }
}
break;
}
@@ -432,8 +450,14 @@
public void serviceDied(long cookie) {
// Deal with service going away
riljLog("serviceDied");
- mRilHandler.sendMessage(mRilHandler.obtainMessage(EVENT_RADIO_PROXY_DEAD,
- HAL_SERVICE_RADIO, 0 /* ignored arg2 */, cookie));
+ if (mFeatureFlags.combineRilDeathHandle()) {
+ mRilHandler.sendMessageAtFrontOfQueue(mRilHandler.obtainMessage(
+ EVENT_RADIO_PROXY_DEAD,
+ HAL_SERVICE_RADIO, 0 /* ignored arg2 */, cookie));
+ } else {
+ mRilHandler.sendMessage(mRilHandler.obtainMessage(EVENT_RADIO_PROXY_DEAD,
+ HAL_SERVICE_RADIO, 0 /* ignored arg2 */, cookie));
+ }
}
}
@@ -465,24 +489,47 @@
@Override
public void binderDied() {
riljLog("Service " + serviceToString(mService) + " has died.");
- mRilHandler.sendMessage(mRilHandler.obtainMessage(EVENT_AIDL_PROXY_DEAD, mService,
- 0 /* ignored arg2 */, mServiceCookies.get(mService).get()));
+ if (mFeatureFlags.combineRilDeathHandle()) {
+ mRilHandler.sendMessageAtFrontOfQueue(mRilHandler.obtainMessage(
+ EVENT_AIDL_PROXY_DEAD, mService, 0 /* ignored arg2 */,
+ mServiceCookies.get(mService).get()));
+ } else {
+ mRilHandler.sendMessage(mRilHandler.obtainMessage(EVENT_AIDL_PROXY_DEAD, mService,
+ 0 /* ignored arg2 */, mServiceCookies.get(mService).get()));
+ }
unlinkToDeath();
}
}
- private synchronized void resetProxyAndRequestList(int service) {
+ /**
+ * Reset services. If one of the AIDL service is reset, all the other AIDL services will be
+ * reset as well.
+ * @param service The service to reset.
+ */
+ private synchronized void resetProxyAndRequestList(@HalService int service) {
if (service == HAL_SERVICE_RADIO) {
mRadioProxy = null;
} else {
- mServiceProxies.get(service).clear();
+ if (mFeatureFlags.combineRilDeathHandle()) {
+ // Reset all aidl services.
+ for (int i = MIN_SERVICE_IDX; i <= MAX_SERVICE_IDX; i++) {
+ if (i == HAL_SERVICE_RADIO) continue;
+ if (mServiceProxies.get(i) == null) {
+ // This should only happen in tests
+ riljLoge("Null service proxy for service " + serviceToString(i));
+ continue;
+ }
+ mServiceProxies.get(i).clear();
+ // Increment the cookie so that death notification can be ignored
+ mServiceCookies.get(i).incrementAndGet();
+ }
+ } else {
+ mServiceProxies.get(service).clear();
+ // Increment the cookie so that death notification can be ignored
+ mServiceCookies.get(service).incrementAndGet();
+ }
}
- // Increment the cookie so that death notification can be ignored
- mServiceCookies.get(service).incrementAndGet();
-
- // TODO: If a service doesn't exist or is unimplemented, it shouldn't cause the radio to
- // become unavailable for all other services
setRadioState(TelephonyManager.RADIO_POWER_UNAVAILABLE, true /* forceNotifyRegistrants */);
RILRequest.resetSerial();
@@ -492,7 +539,20 @@
if (service == HAL_SERVICE_RADIO) {
getRadioProxy();
} else {
- getRadioServiceProxy(service);
+ if (mFeatureFlags.combineRilDeathHandle()) {
+ // Reset all aidl services.
+ for (int i = MIN_SERVICE_IDX; i <= MAX_SERVICE_IDX; i++) {
+ if (i == HAL_SERVICE_RADIO) continue;
+ if (mServiceProxies.get(i) == null) {
+ // This should only happen in tests
+ riljLoge("Null service proxy for service " + serviceToString(i));
+ continue;
+ }
+ getRadioServiceProxy(i);
+ }
+ } else {
+ getRadioServiceProxy(service);
+ }
}
}
@@ -510,10 +570,6 @@
mMockModem = null;
mMockModem = new MockModem(mContext, serviceName, mPhoneId);
- if (mMockModem == null) {
- riljLoge("MockModem create fail.");
- return false;
- }
// Disable HIDL service
if (mRadioProxy != null) {
@@ -550,8 +606,14 @@
if (serviceBound) {
mIsRadioProxyInitialized = false;
- for (int service = MIN_SERVICE_IDX; service <= MAX_SERVICE_IDX; service++) {
- resetProxyAndRequestList(service);
+ if (mFeatureFlags.combineRilDeathHandle()) {
+ // Reset both hidl and aidl proxies.
+ resetProxyAndRequestList(HAL_SERVICE_RADIO);
+ resetProxyAndRequestList(HAL_SERVICE_DATA);
+ } else {
+ for (int service = MIN_SERVICE_IDX; service <= MAX_SERVICE_IDX; service++) {
+ resetProxyAndRequestList(service);
+ }
}
}
}
@@ -579,7 +641,15 @@
mHalVersion.put(service, RADIO_HAL_VERSION_UNSUPPORTED);
}
}
- resetProxyAndRequestList(service);
+ if (!mFeatureFlags.combineRilDeathHandle()) {
+ resetProxyAndRequestList(service);
+ }
+ }
+ if (mFeatureFlags.combineRilDeathHandle()) {
+ // Reset both hidl and aidl proxies. Must be after cleaning mocked halVersion,
+ // otherwise an aidl service will be incorrectly considered as disabled.
+ resetProxyAndRequestList(HAL_SERVICE_RADIO);
+ resetProxyAndRequestList(HAL_SERVICE_DATA);
}
}
}
@@ -988,16 +1058,33 @@
@Override
public synchronized void onSlotActiveStatusChange(boolean active) {
mIsRadioProxyInitialized = false;
- for (int service = MIN_SERVICE_IDX; service <= MAX_SERVICE_IDX; service++) {
+ if (mFeatureFlags.combineRilDeathHandle()) {
if (active) {
- // Try to connect to RIL services and set response functions.
- if (service == HAL_SERVICE_RADIO) {
- getRadioProxy();
- } else {
- getRadioServiceProxy(service);
+ for (int service = MIN_SERVICE_IDX; service <= MAX_SERVICE_IDX; service++) {
+ // Try to connect to RIL services and set response functions.
+ if (service == HAL_SERVICE_RADIO) {
+ getRadioProxy();
+ } else {
+ getRadioServiceProxy(service);
+ }
}
} else {
- resetProxyAndRequestList(service);
+ // Reset both hidl and aidl proxies
+ resetProxyAndRequestList(HAL_SERVICE_RADIO);
+ resetProxyAndRequestList(HAL_SERVICE_DATA);
+ }
+ } else {
+ for (int service = MIN_SERVICE_IDX; service <= MAX_SERVICE_IDX; service++) {
+ if (active) {
+ // Try to connect to RIL services and set response functions.
+ if (service == HAL_SERVICE_RADIO) {
+ getRadioProxy();
+ } else {
+ getRadioServiceProxy(service);
+ }
+ } else {
+ resetProxyAndRequestList(service);
+ }
}
}
}
@@ -1005,19 +1092,16 @@
//***** Constructors
@UnsupportedAppUsage
- public RIL(Context context, int allowedNetworkTypes, int cdmaSubscription) {
- this(context, allowedNetworkTypes, cdmaSubscription, null);
- }
-
- @UnsupportedAppUsage
- public RIL(Context context, int allowedNetworkTypes, int cdmaSubscription, Integer instanceId) {
- this(context, allowedNetworkTypes, cdmaSubscription, instanceId, null);
+ public RIL(Context context, int allowedNetworkTypes, int cdmaSubscription, Integer instanceId,
+ @NonNull FeatureFlags flags) {
+ this(context, allowedNetworkTypes, cdmaSubscription, instanceId, null, flags);
}
@VisibleForTesting
public RIL(Context context, int allowedNetworkTypes, int cdmaSubscription, Integer instanceId,
- SparseArray<RadioServiceProxy> proxies) {
+ SparseArray<RadioServiceProxy> proxies, @NonNull FeatureFlags flags) {
super(context);
+ mFeatureFlags = flags;
if (RILJ_LOGD) {
riljLog("RIL: init allowedNetworkTypes=" + allowedNetworkTypes
+ " cdmaSubscription=" + cdmaSubscription + ")");
diff --git a/tests/telephonytests/src/com/android/internal/telephony/RILTest.java b/tests/telephonytests/src/com/android/internal/telephony/RILTest.java
index bfe9649..88c5389 100644
--- a/tests/telephonytests/src/com/android/internal/telephony/RILTest.java
+++ b/tests/telephonytests/src/com/android/internal/telephony/RILTest.java
@@ -330,7 +330,7 @@
proxies.put(HAL_SERVICE_MODEM, mRadioModemProxy);
mRILInstance = new RIL(context,
RadioAccessFamily.getRafFromNetworkType(RILConstants.PREFERRED_NETWORK_MODE),
- Phone.PREFERRED_CDMA_SUBSCRIPTION, 0, proxies);
+ Phone.PREFERRED_CDMA_SUBSCRIPTION, 0, proxies, mFeatureFlags);
mRILUnderTest = spy(mRILInstance);
doReturn(mRadioProxy).when(mRILUnderTest).getRadioProxy();
doReturn(mDataProxy).when(mRILUnderTest).getRadioServiceProxy(eq(RadioDataProxy.class));
@@ -354,6 +354,8 @@
replaceInstance(RIL.class, "mHalVersion", mRILUnderTest, mHalVersionV14);
} catch (Exception e) {
}
+
+ doReturn(true).when(mFeatureFlags).combineRilDeathHandle();
}
@After