Skip to content

Commit 87e3a3b

Browse files
jkasten2jkasten2
committed
ConcurrentModificationException fix
* Fixed rare issue where ConcurrentModificationException could be thrown from sendTags or other calls made through or from within the OneSignalStateSynchronizer.
1 parent 74e7c48 commit 87e3a3b

File tree

3 files changed

+101
-52
lines changed

3 files changed

+101
-52
lines changed

‎OneSignalSDK/app/src/test/java/com/test/onesignal/MainOneSignalClassRunner.java

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,9 @@
6969
import org.robolectric.util.ActivityController;
7070

7171
import java.lang.reflect.Field;
72+
import java.util.ArrayList;
7273
import java.util.Arrays;
74+
import java.util.List;
7375

7476
@Config(packageName = "com.onesignal.example",
7577
constants = BuildConfig.class,
@@ -631,6 +633,49 @@ public void testSendTagNonStringValues() throws Exception {
631633
Assert.assertFalse(lastGetTags.has("object"));
632634
}
633635

636+
static boolean failedCurModTest;
637+
@Test
638+
public void testSendTagsConcurrentModificationException() throws Exception {
639+
OneSignalInit();
640+
threadAndTaskWait();
641+
642+
for(int a = 0; a < 10; a++) {
643+
List<Thread> threadList = new ArrayList<>(30);
644+
for (int i = 0; i < 30; i++) {
645+
Thread lastThread = newSendTagTestThread(Thread.currentThread(), i);
646+
lastThread.start();
647+
threadList.add(lastThread);
648+
Assert.assertFalse(failedCurModTest);
649+
}
650+
651+
for(Thread thread : threadList)
652+
thread.join();
653+
Assert.assertFalse(failedCurModTest);
654+
}
655+
}
656+
657+
private static Thread newSendTagTestThread(final Thread mainThread, final int id) {
658+
return new Thread(new Runnable() {
659+
@Override
660+
public void run() {
661+
try {
662+
for (int i = 0; i < 100; i++) {
663+
if (failedCurModTest)
664+
break;
665+
OneSignal.sendTags("{\"key" + id + "\": " + i + "}");
666+
}
667+
} catch (Throwable t) {
668+
// Ignore the flaky Robolectric null error.
669+
if (t.getStackTrace()[0].getClassName().equals("org.robolectric.shadows.ShadowMessageQueue"))
670+
return;
671+
failedCurModTest = true;
672+
mainThread.interrupt();
673+
throw t;
674+
}
675+
}
676+
});
677+
}
678+
634679
@Test
635680
public void shouldSaveToSyncIfKilledBeforeDelayedCompare() throws Exception {
636681
OneSignalInit();

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ public void init() {
160160
private static TrackGooglePurchase trackGooglePurchase;
161161
private static TrackAmazonPurchase trackAmazonPurchase;
162162

163-
public static final String VERSION = "020401";
163+
public static final String VERSION = "020402";
164164

165165
private static AdvertisingIdentifierProvider mainAdIdProvider = new AdvertisingIdProviderGPS();
166166

‎OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignalStateSynchronizer.java

Lines changed: 55 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -69,49 +69,51 @@ static private JSONObject generateJsonDiff(JSONObject cur, JSONObject changedTo,
6969
else
7070
output = new JSONObject();
7171

72-
while (keys.hasNext()) {
73-
try {
74-
key = keys.next();
75-
value = changedTo.get(key);
76-
77-
if (cur.has(key)) {
78-
if (value instanceof JSONObject) {
79-
JSONObject curValue = cur.getJSONObject(key);
80-
JSONObject outValue = null;
81-
if (baseOutput != null && baseOutput.has(key))
82-
outValue = baseOutput.getJSONObject(key);
83-
JSONObject returnedJson = generateJsonDiff(curValue, (JSONObject) value, outValue, includeFields);
84-
String returnedJsonStr = returnedJson.toString();
85-
if (!returnedJsonStr.equals("{}"))
86-
output.put(key, new JSONObject(returnedJsonStr));
87-
}
88-
else if (value instanceof JSONArray)
89-
handleJsonArray(key, (JSONArray) value, cur.getJSONArray(key), output);
90-
else if (includeFields != null && includeFields.contains(key))
91-
output.put(key, value);
92-
else {
93-
Object curValue = cur.get(key);
94-
if (!value.equals(curValue)) {
95-
// Work around for JSON serializer turning doubles/floats into ints since it drops ending 0's
96-
if (curValue instanceof Integer && !"".equals(value)) {
97-
if ( ((Number)curValue).doubleValue() != ((Number)value).doubleValue())
72+
synchronized (cur) {
73+
while (keys.hasNext()) {
74+
try {
75+
key = keys.next();
76+
value = changedTo.get(key);
77+
78+
if (cur.has(key)) {
79+
if (value instanceof JSONObject) {
80+
JSONObject curValue = cur.getJSONObject(key);
81+
JSONObject outValue = null;
82+
if (baseOutput != null && baseOutput.has(key))
83+
outValue = baseOutput.getJSONObject(key);
84+
JSONObject returnedJson = generateJsonDiff(curValue, (JSONObject) value, outValue, includeFields);
85+
String returnedJsonStr = returnedJson.toString();
86+
if (!returnedJsonStr.equals("{}"))
87+
output.put(key, new JSONObject(returnedJsonStr));
88+
}
89+
else if (value instanceof JSONArray)
90+
handleJsonArray(key, (JSONArray) value, cur.getJSONArray(key), output);
91+
else if (includeFields != null && includeFields.contains(key))
92+
output.put(key, value);
93+
else {
94+
Object curValue = cur.get(key);
95+
if (!value.equals(curValue)) {
96+
// Work around for JSON serializer turning doubles/floats into ints since it drops ending 0's
97+
if (curValue instanceof Integer && !"".equals(value)) {
98+
if ( ((Number)curValue).doubleValue() != ((Number)value).doubleValue())
99+
output.put(key, value);
100+
}
101+
else
98102
output.put(key, value);
99103
}
100-
else
101-
output.put(key, value);
102104
}
103105
}
106+
else {
107+
if (value instanceof JSONObject)
108+
output.put(key, new JSONObject(value.toString()));
109+
else if (value instanceof JSONArray)
110+
handleJsonArray(key, (JSONArray) value, null, output);
111+
else
112+
output.put(key, value);
113+
}
114+
} catch (JSONException e) {
115+
e.printStackTrace();
104116
}
105-
else {
106-
if (value instanceof JSONObject)
107-
output.put(key, new JSONObject(value.toString()));
108-
else if (value instanceof JSONArray)
109-
handleJsonArray(key, (JSONArray)value, null, output);
110-
else
111-
output.put(key, value);
112-
}
113-
} catch (JSONException e) {
114-
e.printStackTrace();
115117
}
116118
}
117119

@@ -165,22 +167,24 @@ private static JSONObject getTagsWithoutDeletedKeys(JSONObject jsonObject) {
165167
if (jsonObject.has("tags")) {
166168
JSONObject toReturn = new JSONObject();
167169

168-
JSONObject keyValues = jsonObject.optJSONObject("tags");
170+
synchronized (jsonObject) {
171+
JSONObject keyValues = jsonObject.optJSONObject("tags");
169172

170-
Iterator<String> keys = keyValues.keys();
171-
String key;
172-
Object value;
173+
Iterator<String> keys = keyValues.keys();
174+
String key;
175+
Object value;
173176

174-
while (keys.hasNext()) {
175-
key = keys.next();
176-
try {
177-
value = keyValues.get(key);
178-
if (!"".equals(value))
179-
toReturn.put(key, value);
180-
} catch (Throwable t) {}
181-
}
177+
while (keys.hasNext()) {
178+
key = keys.next();
179+
try {
180+
value = keyValues.get(key);
181+
if (!"".equals(value))
182+
toReturn.put(key, value);
183+
} catch (Throwable t) {}
184+
}
182185

183-
return toReturn;
186+
return toReturn;
187+
}
184188
}
185189

186190
return null;

0 commit comments

Comments
 (0)