Skip to content

Commit 84618a6

Browse files
committed
Fix ANR
• In rare situations, the main UI thread can block on the user state synchronization making a potentially very long running HTTP request • The user state synchronization model of the Android SDK needs to be significantly refactored & simplified • In the mean time, this commit makes a minor change so that the user state synchronizer never blocks/holds locks while making HTTP requests
1 parent 58827dc commit 84618a6

File tree

1 file changed

+37
-28
lines changed

1 file changed

+37
-28
lines changed

OneSignalSDK/onesignal/src/main/java/com/onesignal/UserStateSynchronizer.java

Lines changed: 37 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ private boolean syncEmailLogout() {
163163
return getToSyncUserState().dependValues.optBoolean("logoutEmail", false);
164164
}
165165

166-
synchronized void syncUserState(boolean fromSyncService) {
166+
void syncUserState(boolean fromSyncService) {
167167
runningSyncUserState.set(true);
168168
internalSyncUserState(fromSyncService);
169169
runningSyncUserState.set(false);
@@ -287,10 +287,12 @@ private void doPutSync(String userId, final JSONObject jsonBody, final JSONObjec
287287
void onFailure(int statusCode, String response, Throwable throwable) {
288288
OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "Failed last request. statusCode: " + statusCode + "\nresponse: " + response);
289289

290-
if (response400WithErrorsContaining(statusCode, response, "No user with this id found"))
291-
handlePlayerDeletedFromServer();
292-
else
293-
handleNetworkFailure();
290+
synchronized (syncLock) {
291+
if (response400WithErrorsContaining(statusCode, response, "No user with this id found"))
292+
handlePlayerDeletedFromServer();
293+
else
294+
handleNetworkFailure();
295+
}
294296

295297
if (jsonBody.has("tags"))
296298
for (ChangeTagsUpdateHandler handler : tagsHandlers) {
@@ -303,8 +305,11 @@ void onFailure(int statusCode, String response, Throwable throwable) {
303305

304306
@Override
305307
void onSuccess(String response) {
306-
currentUserState.persistStateAfterSync(dependDiff, jsonBody);
307-
onSuccessfulSync(jsonBody);
308+
synchronized (syncLock) {
309+
currentUserState.persistStateAfterSync(dependDiff, jsonBody);
310+
onSuccessfulSync(jsonBody);
311+
}
312+
308313
JSONObject tags = OneSignalStateSynchronizer.getTags(false).result;
309314

310315
if (jsonBody.has("tags") && tags != null)
@@ -330,36 +335,40 @@ private void doCreateOrNewSession(final String userId, final JSONObject jsonBody
330335
OneSignalRestClient.postSync(urlStr, jsonBody, new OneSignalRestClient.ResponseHandler() {
331336
@Override
332337
void onFailure(int statusCode, String response, Throwable throwable) {
333-
waitingForSessionResponse = false;
334-
OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "Failed last request. statusCode: " + statusCode + "\nresponse: " + response);
338+
synchronized (syncLock) {
339+
waitingForSessionResponse = false;
340+
OneSignal.Log(OneSignal.LOG_LEVEL.WARN, "Failed last request. statusCode: " + statusCode + "\nresponse: " + response);
335341

336-
if (response400WithErrorsContaining(statusCode, response, "not a valid device_type"))
337-
handlePlayerDeletedFromServer();
338-
else
339-
handleNetworkFailure();
342+
if (response400WithErrorsContaining(statusCode, response, "not a valid device_type"))
343+
handlePlayerDeletedFromServer();
344+
else
345+
handleNetworkFailure();
346+
}
340347
}
341348

342349
@Override
343350
void onSuccess(String response) {
344-
nextSyncIsSession = waitingForSessionResponse = false;
345-
currentUserState.persistStateAfterSync(dependDiff, jsonBody);
351+
synchronized (syncLock) {
352+
nextSyncIsSession = waitingForSessionResponse = false;
353+
currentUserState.persistStateAfterSync(dependDiff, jsonBody);
346354

347-
try {
348-
JSONObject jsonResponse = new JSONObject(response);
355+
try {
356+
JSONObject jsonResponse = new JSONObject(response);
349357

350-
if (jsonResponse.has("id")) {
351-
String newUserId = jsonResponse.optString("id");
352-
updateIdDependents(newUserId);
358+
if (jsonResponse.has("id")) {
359+
String newUserId = jsonResponse.optString("id");
360+
updateIdDependents(newUserId);
353361

354-
OneSignal.Log(OneSignal.LOG_LEVEL.INFO, "Device registered, UserId = " + newUserId);
355-
}
356-
else
357-
OneSignal.Log(OneSignal.LOG_LEVEL.INFO, "session sent, UserId = " + userId);
362+
OneSignal.Log(OneSignal.LOG_LEVEL.INFO, "Device registered, UserId = " + newUserId);
363+
}
364+
else
365+
OneSignal.Log(OneSignal.LOG_LEVEL.INFO, "session sent, UserId = " + userId);
358366

359-
OneSignal.updateOnSessionDependents();
360-
onSuccessfulSync(jsonBody);
361-
} catch (Throwable t) {
362-
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "ERROR parsing on_session or create JSON Response.", t);
367+
OneSignal.updateOnSessionDependents();
368+
onSuccessfulSync(jsonBody);
369+
} catch (Throwable t) {
370+
OneSignal.Log(OneSignal.LOG_LEVEL.ERROR, "ERROR parsing on_session or create JSON Response.", t);
371+
}
363372
}
364373
}
365374
});

0 commit comments

Comments
 (0)