Skip to content

Commit 41b9e16

Browse files
authored
Merge pull request #2736 from DataDog/yl/sr-concurrent-modification-fix
RUM-10283: Shallow copy node wireframes before iterating in NodeFlattener
2 parents 7e96c45 + ed951ac commit 41b9e16

File tree

2 files changed

+43
-2
lines changed
  • features/dd-sdk-android-session-replay/src

2 files changed

+43
-2
lines changed

features/dd-sdk-android-session-replay/src/main/kotlin/com/datadog/android/sessionreplay/internal/processor/NodeFlattener.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ internal class NodeFlattener(private val wireframeUtils: WireframeUtils = Wirefr
1919
stack.push(root)
2020
while (stack.isNotEmpty()) {
2121
val node = stack.pop()
22-
node.wireframes
22+
node.wireframes.toList()
2323
.map { wireframe ->
2424
val clip = wireframeUtils.resolveWireframeClip(wireframe, node.parents)
2525
wireframe.copy(clip = clip)

features/dd-sdk-android-session-replay/src/test/kotlin/com/datadog/android/sessionreplay/internal/processor/NodeFlattenerTest.kt

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import fr.xgouchet.elmyr.junit5.ForgeExtension
1515
import org.assertj.core.api.Assertions.assertThat
1616
import org.junit.jupiter.api.BeforeEach
1717
import org.junit.jupiter.api.Test
18+
import org.junit.jupiter.api.assertDoesNotThrow
1819
import org.junit.jupiter.api.extension.ExtendWith
1920
import org.junit.jupiter.api.extension.Extensions
2021
import org.mockito.Mock
@@ -25,7 +26,6 @@ import org.mockito.kotlin.eq
2526
import org.mockito.kotlin.whenever
2627
import org.mockito.quality.Strictness
2728
import java.util.LinkedList
28-
import kotlin.collections.ArrayList
2929
import kotlin.math.pow
3030

3131
@Extensions(
@@ -135,10 +135,51 @@ internal class NodeFlattenerTest {
135135
assertThat(wireframes).isEqualTo(expectedList)
136136
}
137137

138+
@Test
139+
fun `M not have ConcurrentModificationException W modifying nodes concurrently`(forge: Forge) {
140+
// Given
141+
val maxWidth = forge.aLong(2, 1000)
142+
val maxHeight = forge.aLong(2, 1000)
143+
val wireframeSize = forge.anInt(min = 10, max = 100)
144+
val expectedList: MutableList<MobileSegment.Wireframe> =
145+
Array<MobileSegment.Wireframe>(wireframeSize) {
146+
// just to avoid collisions we will add the wireframes manually
147+
val width = forge.aLong(min = 1, max = maxWidth)
148+
val height = forge.aLong(min = 1, max = maxHeight)
149+
val x = it * maxWidth
150+
val y = it * maxHeight
151+
forge.getForgery<MobileSegment.Wireframe.ShapeWireframe>()
152+
.copy(width = width, height = height, x = x, y = y)
153+
}.toMutableList()
154+
val fakeSnapshot = generateFlattenNodeFromList(expectedList)
155+
156+
// When
157+
val thread = Thread {
158+
repeat(forge.anInt(1, wireframeSize - 1)) {
159+
expectedList.removeAt(0)
160+
}
161+
}
162+
163+
// Then
164+
assertDoesNotThrow {
165+
thread.start()
166+
testedNodeFlattener.flattenNode(fakeSnapshot)
167+
thread.join()
168+
}
169+
}
170+
138171
// endregion
139172

140173
// region Internals
141174

175+
private fun generateFlattenNodeFromList(list: List<MobileSegment.Wireframe>): Node {
176+
return Node(
177+
wireframes = list,
178+
children = emptyList(),
179+
parents = emptyList()
180+
)
181+
}
182+
142183
private fun generateTreeFromList(list: List<MobileSegment.Wireframe>): Node {
143184
val mutableList = list.toMutableList()
144185
val root = mutableList.removeAt(0).toNode()

0 commit comments

Comments
 (0)