Skip to content

Commit 20d04ae

Browse files
nickcernerameta-codesync[bot]
authored andcommitted
Fix ConcurrentModificationException in IntentModule.getInitialURL re-entrancy (#57667)
Summary: `IntentModule.onHostResume()` iterates `pendingOpenURLPromises` (an `ArrayList`) directly and calls `getInitialURL()` for each pending promise. When `getCurrentActivity()` returns `null` at that moment — e.g. a deep link or notification tap landing mid activity-transition during a rapid pause/resume — `getInitialURL()` re-enters `waitForActivityAndGetInitialURL()`, which calls `pendingOpenURLPromises.add(promise)` on the very list being iterated. The next iteration then throws `java.util.ConcurrentModificationException`: ``` java.util.ConcurrentModificationException at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013) at java.util.ArrayList$Itr.next(ArrayList.java:967) at com.facebook.react.modules.intent.IntentModule$waitForActivityAndGetInitialURL$1.onHostResume(IntentModule.kt:90) ``` The `synchronized(this@IntentModule)` guard does not prevent this: the re-entrancy is on the same thread, which already holds the (reentrant) lock, so no second lock acquisition happens. The crash is the `ArrayList` iterator's `modCount` check, not a cross-thread race. The fix snapshots the pending promises into a local copy, clears the shared list, and nulls the listener **before** draining. Re-queued promises then land in the now-empty `pendingOpenURLPromises` and register a fresh listener for the next resume, instead of mutating the list being iterated. Behaviour is otherwise unchanged. ## Changelog: [ANDROID] [FIXED] - Fix ConcurrentModificationException when getInitialURL re-enters during onHostResume Pull Request resolved: #57667 Test Plan: Added `IntentModuleTest.getInitialURL_onHostResumeWithNullActivity_doesNotThrowAndPreservesPromise`, a Robolectric regression test that registers a pending promise while the current activity is `null`, then drives `onHostResume` so the drain re-queues, and asserts the drain does not throw and the promise is preserved (neither resolved nor rejected). Ran locally against `main` with the exact reproduction scenario: **With the fix** — passes: ``` $ ./gradlew :packages:react-native:ReactAndroid:testDebugUnitTest \ --tests "com.facebook.react.modules.intent.IntentModuleTest" BUILD SUCCESSFUL # tests=1, failures=0, errors=0 ``` **Without the fix** (reverting `IntentModule.kt` only) — fails with the exact crash, confirming the test is a genuine regression test: ``` IntentModuleTest > getInitialURL_onHostResumeWithNullActivity_doesNotThrowAndPreservesPromise FAILED java.util.ConcurrentModificationException at java.util.ArrayList$Itr.checkForComodification(ArrayList.java:1013) at java.util.ArrayList$Itr.next(ArrayList.java:967) at com.facebook.react.modules.intent.IntentModule$waitForActivityAndGetInitialURL$1.onHostResume(IntentModule.kt:90) ``` `ktfmt` (Meta style, matching the repo's `ktfmt` configuration) reports both changed files as already formatted. Reviewed By: cortinico Differential Revision: D113595327 Pulled By: fabriziocucci fbshipit-source-id: b1247cb6e473fd7c550dda8b2b584d720ea3e46c
1 parent c1843c4 commit 20d04ae

2 files changed

Lines changed: 150 additions & 3 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/modules/intent/IntentModule.kt

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -87,11 +87,15 @@ public open class IntentModule(reactContext: ReactApplicationContext) :
8787
override fun onHostResume() {
8888
reactApplicationContext.removeLifecycleEventListener(this)
8989
synchronized(this@IntentModule) {
90-
for (pendingPromise in pendingOpenURLPromises) {
90+
// getInitialURL can re-enter and re-add to pendingOpenURLPromises when the activity
91+
// is still null at resume, so drain a snapshot (after clearing the list and listener)
92+
// to avoid mutating the list being iterated (ConcurrentModificationException).
93+
val pendingPromises = ArrayList(pendingOpenURLPromises)
94+
pendingOpenURLPromises.clear()
95+
initialURLListener = null
96+
for (pendingPromise in pendingPromises) {
9197
getInitialURL(pendingPromise)
9298
}
93-
initialURLListener = null
94-
pendingOpenURLPromises.clear()
9599
}
96100
}
97101

Lines changed: 143 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,143 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
package com.facebook.react.modules.intent
9+
10+
import com.facebook.react.bridge.LifecycleEventListener
11+
import com.facebook.react.bridge.Promise
12+
import com.facebook.react.bridge.ReactApplicationContext
13+
import com.facebook.react.bridge.WritableMap
14+
import org.assertj.core.api.Assertions.assertThat
15+
import org.assertj.core.api.Assertions.assertThatCode
16+
import org.junit.Before
17+
import org.junit.Test
18+
import org.junit.runner.RunWith
19+
import org.mockito.kotlin.any
20+
import org.mockito.kotlin.argumentCaptor
21+
import org.mockito.kotlin.mock
22+
import org.mockito.kotlin.times
23+
import org.mockito.kotlin.verify
24+
import org.mockito.kotlin.whenever
25+
import org.robolectric.RobolectricTestRunner
26+
27+
@RunWith(RobolectricTestRunner::class)
28+
class IntentModuleTest {
29+
30+
private lateinit var context: ReactApplicationContext
31+
private lateinit var intentModule: IntentModule
32+
33+
@Before
34+
fun setUp() {
35+
context = mock<ReactApplicationContext>()
36+
intentModule = IntentModule(context)
37+
}
38+
39+
/**
40+
* Regression test for the ConcurrentModificationException thrown from onHostResume. When the
41+
* current activity is still null at resume time, draining pendingOpenURLPromises re-enters
42+
* getInitialURL -> waitForActivityAndGetInitialURL, which adds back into the same list. Before
43+
* the fix this mutated the list mid-iteration and crashed. The synchronized guard does not help:
44+
* the re-entrancy is on the same thread and already holds the lock.
45+
*/
46+
@Test
47+
fun getInitialURL_onHostResumeWithNullActivity_doesNotThrowAndPreservesPromise() {
48+
// No current activity: getInitialURL queues the promise and registers a lifecycle listener.
49+
whenever(context.currentActivity).thenReturn(null)
50+
51+
val promise = SimplePromise()
52+
intentModule.getInitialURL(promise)
53+
54+
val listenerCaptor = argumentCaptor<LifecycleEventListener>()
55+
verify(context).addLifecycleEventListener(listenerCaptor.capture())
56+
57+
// Resume while the activity is still null (e.g. a deep link landing mid activity-transition).
58+
// The drain re-queues the promise; before the fix this threw ConcurrentModificationException.
59+
assertThatCode { listenerCaptor.firstValue.onHostResume() }.doesNotThrowAnyException()
60+
61+
// The promise was re-queued rather than silently dropped: a fresh listener is registered for
62+
// the next resume, and the promise is left pending (neither resolved nor rejected).
63+
verify(context, times(2)).addLifecycleEventListener(any())
64+
assertThat(promise.resolved).isEqualTo(0)
65+
assertThat(promise.rejected).isEqualTo(0)
66+
}
67+
68+
internal class SimplePromise : Promise {
69+
companion object {
70+
private const val ERROR_DEFAULT_CODE = "EUNSPECIFIED"
71+
private const val ERROR_DEFAULT_MESSAGE = "Error not specified."
72+
}
73+
74+
var resolved = 0
75+
private set
76+
77+
var rejected = 0
78+
private set
79+
80+
var value: Any? = null
81+
private set
82+
83+
var errorCode: String? = null
84+
private set
85+
86+
var errorMessage: String? = null
87+
private set
88+
89+
override fun resolve(value: Any?) {
90+
resolved++
91+
this.value = value
92+
}
93+
94+
override fun reject(code: String?, message: String?) {
95+
reject(code, message, null, null)
96+
}
97+
98+
override fun reject(code: String?, throwable: Throwable?) {
99+
reject(code, null, throwable, null)
100+
}
101+
102+
override fun reject(code: String?, message: String?, throwable: Throwable?) {
103+
reject(code, message, throwable, null)
104+
}
105+
106+
override fun reject(throwable: Throwable) {
107+
reject(null, null, throwable, null)
108+
}
109+
110+
override fun reject(throwable: Throwable, userInfo: WritableMap) {
111+
reject(null, null, throwable, userInfo)
112+
}
113+
114+
override fun reject(code: String?, userInfo: WritableMap) {
115+
reject(code, null, null, userInfo)
116+
}
117+
118+
override fun reject(code: String?, throwable: Throwable?, userInfo: WritableMap) {
119+
reject(code, null, throwable, userInfo)
120+
}
121+
122+
override fun reject(code: String?, message: String?, userInfo: WritableMap) {
123+
reject(code, message, null, userInfo)
124+
}
125+
126+
override fun reject(
127+
code: String?,
128+
message: String?,
129+
throwable: Throwable?,
130+
userInfo: WritableMap?,
131+
) {
132+
rejected++
133+
134+
errorCode = code ?: ERROR_DEFAULT_CODE
135+
errorMessage = message ?: throwable?.message ?: ERROR_DEFAULT_MESSAGE
136+
}
137+
138+
@Deprecated("Method deprecated", ReplaceWith("reject(code, message)"))
139+
override fun reject(message: String) {
140+
reject(null, message, null, null)
141+
}
142+
}
143+
}

0 commit comments

Comments
 (0)