Skip to content

Commit ee36352

Browse files
committed
fix(telephony/legacy): handle potential NPE crash on SIM state change request
To reproduce this: 1. Need to unset/disable the PIN code for all SIM cards 2. Assiduously spam the on/off toggle for all SIM cards The fix consists of two parts: 1. We must separately synchronize the call to setSimState(). The NPE happened, because, multiple calls can be performed in parallel (see line 1 & 30), and since we're only synchronizing the wait()/notifyAll() calls, when a requests finishes slightly earlier, it cleans up the requests metadata written by the other call. 2. We also want to refrain from handling SIM power state response directly inside callback listeners, as it'll likely be executed on the main thread, while in some circumstances we can make database requests via Room that disallows this and can throw an exception. Also, we don't want to do it before the notifyAll() call, since this is a potential error-prone issue that can trigger a false-positive timeout of the pending request if for some reason it will take too long to complete. Instead, we store the request response code in globally accessible metadata, then notify the caller and handle the response inside the caller's worker thread 1 D 7SIM.SimListViewModel handleOnSimStateChanged(simEntryId=0,enabled=true). 2 D 7SIM.TelephonyController setSimState(slotIndex=0,enabled=true,keepDisabledAcrossBoots=false) : In sync block. 3 V 7SIM.SubscriptionsImplLegacy persistSubscription(sub=Subscription { id=2 slotIndex=0 simState=ENABLED iconTint=-13408298 name=Vodafone (work) lastActivatedTime=2024-08-20T15:38 lastDeactivatedTime=-999999999-01-01T00:00 keepDisabledAcrossBoots=false }). 4 D 7SIM.SubscriptionsImplLegacy persistSubscriptionState(subId=2,state=ENABLED). 5 V 7SIM.SubscriptionsImplLegacy addOnSimStatusChangedListener(). 6 V 7SIM.SubscriptionsImplLegacy registerCarrierConfigChangedReceiver(). 7 V 7SIM.SubscriptionsImplLegacy onReceive() : intent=Intent { act=android.telephony.action.SIM_CARD_STATE_CHANGED flg=0x5000010 (has extras) } 8 V 7SIM.SubscriptionsImplLegacy dispatchOnSimStatusChanged(slotIndex=0,state=11). 9 V 7SIM.TelephonyController onSimStatusChanged(slotIndex=0,state=11). 10 D 7SIM.TelephonyController handleOnSetSimPowerStateForSlotFinished(resCode=11) : requestMetadata=Bundle[{last_activated_time=-999999999-01-01T00:00, last_deactivated_time=2024-08-20T15:38, subscription=Subscription { id=2 slotIndex=0 simState=ENABLED iconTint=-13408298 name=Vodafone (work) lastActivatedTime=2024-08-20T15:38 lastDeactivatedTime=-999999999-01-01T00:00 keepDisabledAcrossBoots=false }, keep_disabled_across_boots=true}], requestFailed=false,shouldNotifyAllListeners=true 11 V 7SIM.SubscriptionsImplLegacy removeOnSimStatusChangedListener(). 12 V 7SIM.SubscriptionsImplLegacy unregisterCarrierConfigChangedReceiver(). 13 D 7SIM.DirectBootAwareBroadcastReceiver onReceive() : intent=Intent { act=android.telephony.action.CARRIER_CONFIG_CHANGED flg=0x15000010 cmp=com.github.iusmac.sevensim/.DirectBootAwareBroadcastReceiver (has extras) } 14 D 7SIM.ForegroundService onCreate(). 15 D 7SIM.ForegroundService onStartCommand(intent=Intent { act=ACTION_SUBSCRIPTIONS_CHANGED cmp=com.github.iusmac.sevensim/.ForegroundService (has extras) },flags=0,startId=1). 16 D 7SIM.ForegroundService Worker.execute(taskId=1) Add : mQueueSize=0. 17 D 7SIM.ForegroundService onStartCommand(intent=Intent { act=ACTION_SYNC_SUBSCRIPTION_ENABLED_STATE cmp=com.github.iusmac.sevensim/.ForegroundService (has extras) },flags=0,startId=2). 18 D 7SIM.ForegroundService Worker.execute(taskId=1) Start : mQueueSize=1. 19 D 7SIM.ForegroundService Worker.execute(taskId=2) Add : mQueueSize=1. 20 D 7SIM.ForegroundService onStartCommand(intent=Intent { act=ACTION_UPDATE_NEXT_WEEKLY_REPEAT_SCHEDULE_PROCESSING_ITER cmp=com.github.iusmac.sevensim/.ForegroundService (has extras) },flags=0,startId=3). 21 D 7SIM.ForegroundService Worker.execute(taskId=3) Add : mQueueSize=2. 22 D 7SIM.SubscriptionsImplLegacy syncSubscriptions(dateTime=2024-08-20T15:38:51.853) : Subscription { id=1 slotIndex=1 simState=ENABLED iconTint=-4056997 name=Vodafone lastActivatedTime=-999999999-01-01T00:00 lastDeactivatedTime=-999999999-01-01T00:00 keepDisabledAcrossBoots=false },currentSubState=ENABLED,expectedSubState=UNKNOWN,existsInUsableList=false. 23 D 7SIM.SubscriptionsImplLegacy persistSubscriptionState(subId=1,state=ENABLED). 24 D 7SIM.SubscriptionsImplLegacy syncSubscriptions(dateTime=2024-08-20T15:38:51.853) : Subscription { id=2 slotIndex=-1 simState=UNKNOWN iconTint=-16777216 name= lastActivatedTime=2024-08-20T15:38 lastDeactivatedTime=-999999999-01-01T00:00 keepDisabledAcrossBoots=false }. 25 V 7SIM.SubscriptionsImplLegacy persistSubscription(sub=Subscription { id=2 slotIndex=-1 simState=UNKNOWN iconTint=-16777216 name= lastActivatedTime=-999999999-01-01T00:00 lastDeactivatedTime=-999999999-01-01T00:00 keepDisabledAcrossBoots=false }). 26 D 7SIM.SubscriptionsImplLegacy persistSubscriptionState(subId=2,state=UNKNOWN). 27 D 7SIM.ForegroundService Worker.execute(taskId=1) Finish : mQueueSize=2. 28 D 7SIM.ForegroundService Worker.execute(taskId=2) Start : mQueueSize=2. 29 D 7SIM.SubscriptionScheduler syncSubscriptionEnabledState(subId=1,compareTime=2024-08-20T15:38:51.853,overrideUserPreference=false) : Subscription { id=1 slotIndex=1 simState=ENABLED iconTint=-4056997 name=Vodafone lastActivatedTime=-999999999-01-01T00:00 lastDeactivatedTime=-999999999-01-01T00:00 keepDisabledAcrossBoots=false },nearestEnableTime=Optional[2024-08-20T14:30],nearestDisableTime=Optional[2024-08-20T18:30],expectedEnabled=false,isInCall=false. 30 D 7SIM.TelephonyController setSimState(slotIndex=1,enabled=false,keepDisabledAcrossBoots=false) : In sync block. 31 V 7SIM.SubscriptionsImplLegacy persistSubscription(sub=Subscription { id=1 slotIndex=1 simState=DISABLED iconTint=-4056997 name=Vodafone lastActivatedTime=-999999999-01-01T00:00 lastDeactivatedTime=2024-08-20T15:38 keepDisabledAcrossBoots=false }). 32 D 7SIM.SubscriptionsImplLegacy persistSubscriptionState(subId=1,state=DISABLED). 33 V 7SIM.SubscriptionsImplLegacy addOnSimStatusChangedListener(). 34 V 7SIM.SubscriptionsImplLegacy registerCarrierConfigChangedReceiver(). 35 D 7SIM.SimListViewModel handleOnSimStateChanged(simEntryId=1,enabled=false). 36 D 7SIM.TelephonyController setSimState(slotIndex=1,enabled=false,keepDisabledAcrossBoots=true) : In sync block. 37 V 7SIM.SubscriptionsImplLegacy onReceive() : intent=Intent { act=android.telephony.action.SIM_CARD_STATE_CHANGED flg=0x5000010 (has extras) } 38 V 7SIM.SubscriptionsImplLegacy dispatchOnSimStatusChanged(slotIndex=1,state=1). 39 V 7SIM.TelephonyController onSimStatusChanged(slotIndex=1,state=1). 40 E AndroidRuntime Process: com.github.iusmac.sevensim, PID: 2426 41 E AndroidRuntime java.lang.RuntimeException: Error receiving broadcast Intent { act=android.telephony.action.SIM_CARD_STATE_CHANGED flg=0x5000010 (has extras) } in com.github.iusmac.sevensim.telephony.Subscriptions$1@a222e8c 42 E AndroidRuntime Caused by: java.lang.NullPointerException: Attempt to invoke virtual method 'int com.github.iusmac.sevensim.telephony.Subscription.getSlotIndex()' on a null object reference 43 E AndroidRuntime at com.github.iusmac.sevensim.telephony.TelephonyController$SimStatusChangedListener.onSimStatusChanged(TelephonyController.java:372) 44 E AndroidRuntime at com.github.iusmac.sevensim.telephony.Subscriptions.dispatchOnSimStatusChanged(Subscriptions.java:446) 45 E AndroidRuntime at com.github.iusmac.sevensim.telephony.Subscriptions.-$$Nest$mdispatchOnSimStatusChanged(Unknown Source:0) 46 E AndroidRuntime at com.github.iusmac.sevensim.telephony.Subscriptions$1.onReceive(Subscriptions.java:85) 47 I am_crash [2426,0,com.github.iusmac.sevensim,550026951,java.lang.NullPointerException,Attempt to invoke virtual method 'int com.github.iusmac.sevensim.telephony.Subscription.getSlotIndex()' on a null object reference,TelephonyController.java,372] Signed-off-by: iusmac <iusico.maxim@libero.it>
1 parent 5ca206f commit ee36352

1 file changed

Lines changed: 59 additions & 44 deletions

File tree

src/com/github/iusmac/sevensim/telephony/TelephonyController.java

Lines changed: 59 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,11 @@ public final class TelephonyController {
6161
private static final String KEY_LAST_ACTIVATED_TIME = "last_activated_time";
6262
private static final String KEY_LAST_DEACTIVATED_TIME = "last_deactivated_time";
6363
private static final String KEY_KEEP_DISABLED_ACROSS_BOOTS = "keep_disabled_across_boots";
64+
private static final String KEY_REQUEST_RESPONSE_CODE = "request_response_code";
6465

6566
/** The globally accessible request metadata used when performing SIM power state mutations. */
66-
@GuardedBy("this")
67-
private final Bundle mRequestMetadata = new Bundle(4);
67+
@GuardedBy("mRequestMetadata")
68+
private final Bundle mRequestMetadata = new Bundle(5);
6869

6970
@GuardedBy("this")
7071
private SimStatusChangedListener mSimStatusChangedListener;
@@ -128,11 +129,10 @@ public void setSimState(final int slotIndex, final boolean enabled,
128129
return;
129130
}
130131

131-
// Globally save metadata needed when handling SIM power change request termination
132-
mRequestMetadata.putParcelable(KEY_SUBSCRIPTION, sub);
133-
if (sub.getKeepDisabledAcrossBoots() != null) {
134-
mRequestMetadata.putBoolean(KEY_KEEP_DISABLED_ACROSS_BOOTS,
135-
sub.getKeepDisabledAcrossBoots());
132+
if (enabled == sub.isSimEnabled()) {
133+
mLogger.w(logPrefix + "Already in state.");
134+
mSubscriptions.notifyAllListeners();
135+
return;
136136
}
137137

138138
// Keep track of SIM state whenever it's mutated. This will be persisted in a volatile
@@ -142,18 +142,26 @@ public void setSimState(final int slotIndex, final boolean enabled,
142142
// SubscriptionManager anymore, even though the SIM is still present in the slot
143143
sub.setSimState(TelephonyUtils.simStateInt(enabled));
144144

145-
mRequestMetadata.putString(KEY_LAST_ACTIVATED_TIME,
146-
sub.getLastActivatedTime().toString());
147-
mRequestMetadata.putString(KEY_LAST_DEACTIVATED_TIME,
148-
sub.getLastDeactivatedTime().toString());
149-
150145
sub.setLastActivatedTime(enabled ? LocalDateTime.now(ZoneId.systemDefault()) :
151146
LocalDateTime.MIN);
152147
sub.setLastDeactivatedTime(!enabled ? LocalDateTime.now(ZoneId.systemDefault()) :
153148
LocalDateTime.MIN);
154149

155150
sub.keepDisabledAcrossBoots(keepDisabledAcrossBoots);
156151

152+
// Globally save metadata needed when handling SIM power change request termination
153+
synchronized (mRequestMetadata) {
154+
mRequestMetadata.putParcelable(KEY_SUBSCRIPTION, sub);
155+
mRequestMetadata.putString(KEY_LAST_ACTIVATED_TIME,
156+
sub.getLastActivatedTime().toString());
157+
mRequestMetadata.putString(KEY_LAST_DEACTIVATED_TIME,
158+
sub.getLastDeactivatedTime().toString());
159+
if (sub.getKeepDisabledAcrossBoots() != null) {
160+
mRequestMetadata.putBoolean(KEY_KEEP_DISABLED_ACROSS_BOOTS,
161+
sub.getKeepDisabledAcrossBoots());
162+
}
163+
}
164+
157165
// Before making any request, persist the subscription associated with the SIM whose
158166
// power state we're going to change, to immediately reflect the changes on the callers
159167
// side, since this request is generally successful and pretty fast. In case the SIM
@@ -175,15 +183,18 @@ public void setSimState(final int slotIndex, final boolean enabled,
175183
long nowMillis = System.currentTimeMillis();
176184
final long deadlineMillis = nowMillis + SET_SIM_POWER_STATE_REQUEST_TIMEOUT_MILLIS;
177185
do {
178-
try {
179-
wait(deadlineMillis - nowMillis);
180-
} catch (InterruptedException e) {
181-
mLogger.w(logPrefix + "Acquire wait interrupted.");
182-
break;
183-
}
184-
if (mRequestMetadata.isEmpty()) {
185-
// Request metadata consumed, so we break here as this wasn't a spurious wakeup
186-
break;
186+
synchronized (mRequestMetadata) {
187+
try {
188+
mRequestMetadata.wait(deadlineMillis - nowMillis);
189+
} catch (InterruptedException e) {
190+
mLogger.w(logPrefix + "Acquire wait interrupted.");
191+
break;
192+
}
193+
if (mRequestMetadata.containsKey(KEY_REQUEST_RESPONSE_CODE)) {
194+
// SIM power request finished, so we break here as this wasn't a spurious
195+
// wakeup nor a timeout
196+
break;
197+
}
187198
}
188199
nowMillis = System.currentTimeMillis();
189200
} while (nowMillis < deadlineMillis);
@@ -193,19 +204,25 @@ public void setSimState(final int slotIndex, final boolean enabled,
193204
mSimStatusChangedListener = null;
194205
}
195206

196-
if (!mRequestMetadata.isEmpty()) { // Timed out
207+
synchronized (mRequestMetadata) {
197208
final int resCode;
198-
if (enabled) {
199-
// When trying to enable SIM, but the response from modem timeouts, then we
200-
// know there's no SIM card in the slot. This is an implicit edge case that
201-
// needs to be handled manually, because by Android telephony design, a powered
202-
// up modem won't respond if there's no SIM card
203-
resCode = SET_SIM_POWER_STATE_SIM_ABSENT;
209+
if (!mRequestMetadata.containsKey(KEY_REQUEST_RESPONSE_CODE)) { // Timed out
210+
if (enabled) {
211+
// When trying to enable SIM, but the response from modem timeouts, then we
212+
// know there's no SIM card in the slot. This is an implicit edge case that
213+
// needs to be handled manually, because by Android telephony design, a
214+
// powered up modem won't respond if there's no SIM card
215+
resCode = SET_SIM_POWER_STATE_SIM_ABSENT;
216+
} else {
217+
// When trying to disable SIM, but the response from modem timeouts, then
218+
// most likely the device modem does not support
219+
// TelephonyManager#setSimPowerStateForSlot call. So far, this can occur on
220+
// non-QCOM SoCs
221+
resCode = SET_SIM_POWER_STATE_MODEM_TIMEOUT;
222+
}
204223
} else {
205-
// When trying to disable SIM, but the response from modem timeouts, then most
206-
// likely the device modem does not support TelephonyManager#setSimPowerStateForSlot
207-
// call. So far, this can occur on non-QCOM SoCs
208-
resCode = SET_SIM_POWER_STATE_MODEM_TIMEOUT;
224+
resCode = mRequestMetadata.getInt(KEY_REQUEST_RESPONSE_CODE);
225+
mRequestMetadata.remove(KEY_REQUEST_RESPONSE_CODE);
209226
}
210227
handleOnSetSimPowerStateForSlotFinished(resCode);
211228
}
@@ -220,17 +237,15 @@ public void setSimState(final int slotIndex, final boolean enabled,
220237
* {@link TelephonyManager#CARD_POWER_UP}
221238
* {@link TelephonyManager#CARD_POWER_DOWN}
222239
*/
223-
@GuardedBy("this")
224240
private void setSimPowerStateForSlot(final int slotIndex, final int state) {
225241
if (Utils.IS_AT_LEAST_S) {
226242
final Consumer<Integer> callback = (resCode) -> {
227-
synchronized (TelephonyController.this) {
228-
handleOnSetSimPowerStateForSlotFinished(resCode);
229-
TelephonyController.this.notifyAll();
243+
synchronized (mRequestMetadata) {
244+
mRequestMetadata.putInt(KEY_REQUEST_RESPONSE_CODE, resCode);
245+
mRequestMetadata.notifyAll();
230246
}
231247
};
232-
mTelephonyManager.setSimPowerStateForSlot(slotIndex, state, (runnable) ->
233-
runnable.run(), callback);
248+
mTelephonyManager.setSimPowerStateForSlot(slotIndex, state, Runnable::run, callback);
234249
} else {
235250
ApiDeprecated.setSimPowerStateForSlot(mTelephonyManager, slotIndex, state);
236251
}
@@ -239,7 +254,8 @@ private void setSimPowerStateForSlot(final int slotIndex, final int state) {
239254
/**
240255
* @param resCode The response code from the modem.
241256
*/
242-
@GuardedBy("this")
257+
@WorkerThread
258+
@GuardedBy("mRequestMetadata")
243259
private void handleOnSetSimPowerStateForSlotFinished(final int resCode) {
244260
final String logPrefix = String.format(Locale.getDefault(),
245261
"handleOnSetSimPowerStateForSlotFinished(resCode=%d) : requestMetadata=%s",
@@ -339,8 +355,7 @@ private void handleOnSetSimPowerStateForSlotFinished(final int resCode) {
339355
mSubscriptions.notifyAllListeners();
340356
}
341357

342-
// Nullify all the metadata as we no longer need them + this will also signal that the
343-
// response has been successfully handled within the timeout interval
358+
// Nullify all the metadata as we no longer need them
344359
mRequestMetadata.clear();
345360
}
346361

@@ -380,7 +395,7 @@ public void onSimStatusChanged(final int slotIndex,
380395
default: return;
381396
}
382397

383-
synchronized (TelephonyController.this) {
398+
synchronized (mRequestMetadata) {
384399
final Subscription sub = BundleCompat.getParcelable(mRequestMetadata,
385400
KEY_SUBSCRIPTION, Subscription.class);
386401

@@ -391,8 +406,8 @@ public void onSimStatusChanged(final int slotIndex,
391406
return;
392407
}
393408

394-
handleOnSetSimPowerStateForSlotFinished(state);
395-
TelephonyController.this.notifyAll();
409+
mRequestMetadata.putInt(KEY_REQUEST_RESPONSE_CODE, state);
410+
mRequestMetadata.notifyAll();
396411
}
397412
}
398413
}

0 commit comments

Comments
 (0)