Skip to content

Commit 001f3ad

Browse files
romtsnclaude
andcommitted
fix(replay): Avoid windowLocation race and bitmap leak in SurfaceView capture
Address two issues flagged by review: 1. windowLocation race — root.getLocationOnScreen(windowLocation) ran on the main thread, but compositeSurfaceViewsAndMask read windowLocation[0]/[1] later from the executor thread. If a new capture cycle started before the compositor ran, the field was overwritten and SurfaceView pixels would composite at the wrong offset. Snapshot into locals (windowX/windowY) at capture time and pass them through, matching the existing svLocation → capturedX/capturedY pattern. 2. Bitmap leak when isClosed in SurfaceView callback — when the strategy closed mid-capture, the path recycled the in-flight svBitmap but skipped onCaptureComplete(), so remaining never reached zero and any sibling bitmaps already stored in captures[] leaked until GC. Now still drive the completion latch on the closed path, and have the compositor's early-return path recycle leftover captures. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bd86dea commit 001f3ad

1 file changed

Lines changed: 21 additions & 3 deletions

File tree

sentry-android-replay/src/main/java/io/sentry/android/replay/screenshot/PixelCopyStrategy.kt

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -157,14 +157,18 @@ internal class PixelCopyStrategy(
157157
surfaceViewNodes: List<ViewHierarchyNode.SurfaceViewHierarchyNode>,
158158
viewHierarchy: ViewHierarchyNode,
159159
) {
160+
// Snapshot the window location into locals so the executor-side compositor reads stable
161+
// values even if a new capture cycle starts and overwrites the field.
160162
root.getLocationOnScreen(windowLocation)
163+
val windowX = windowLocation[0]
164+
val windowY = windowLocation[1]
161165

162166
val captures = arrayOfNulls<SurfaceViewCapture>(surfaceViewNodes.size)
163167
val remaining = AtomicInteger(surfaceViewNodes.size)
164168

165169
fun onCaptureComplete() {
166170
if (remaining.decrementAndGet() == 0) {
167-
compositeSurfaceViewsAndMask(root, captures, viewHierarchy)
171+
compositeSurfaceViewsAndMask(root, captures, viewHierarchy, windowX, windowY)
168172
}
169173
}
170174

@@ -191,6 +195,9 @@ internal class PixelCopyStrategy(
191195
{ copyResult: Int ->
192196
if (isClosed.get()) {
193197
svBitmap.recycle()
198+
// still drive the completion latch so any prior captures get recycled by the
199+
// composite step's early-return path.
200+
onCaptureComplete()
194201
return@request
195202
}
196203
if (copyResult == PixelCopy.SUCCESS) {
@@ -214,11 +221,14 @@ internal class PixelCopyStrategy(
214221
root: View,
215222
captures: Array<SurfaceViewCapture?>,
216223
viewHierarchy: ViewHierarchyNode,
224+
windowX: Int,
225+
windowY: Int,
217226
) {
218227
executor.submit(
219228
ReplayRunnable("screenshot_recorder.composite") {
220229
if (isClosed.get() || screenshot.isRecycled) {
221230
options.logger.log(DEBUG, "PixelCopyStrategy is closed, skipping compositing")
231+
recycleCaptures(captures)
222232
return@ReplayRunnable
223233
}
224234

@@ -234,8 +244,8 @@ internal class PixelCopyStrategy(
234244
capture.bitmap,
235245
capture.x,
236246
capture.y,
237-
windowLocation[0],
238-
windowLocation[1],
247+
windowX,
248+
windowY,
239249
config.scaleFactorX,
240250
config.scaleFactorY,
241251
)
@@ -247,6 +257,14 @@ internal class PixelCopyStrategy(
247257
)
248258
}
249259

260+
private fun recycleCaptures(captures: Array<SurfaceViewCapture?>) {
261+
for (capture in captures) {
262+
if (capture != null && !capture.bitmap.isRecycled) {
263+
capture.bitmap.recycle()
264+
}
265+
}
266+
}
267+
250268
override fun onContentChanged() {
251269
contentChanged.set(true)
252270
}

0 commit comments

Comments
 (0)