Skip to content

Commit 241129a

Browse files
committed
fix(replay): Distinguish inline execution from executor rejection
ReplayExecutorService.submit previously returned null both when the caller was on the worker thread (task ran inline) and when the executor rejected the submission (task did NOT run). Callers had no way to tell them apart. Return a CompletedFuture sentinel for inline execution; null now means only rejection. Also narrow PixelCopyStrategy's frame-processing catch from Throwable to RuntimeException so OOM/LinkageError still propagate.
1 parent aee9b30 commit 241129a

2 files changed

Lines changed: 32 additions & 5 deletions

File tree

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ internal class PixelCopyStrategy(
159159
resetUnstableCaptures = !changedDuringCapture,
160160
)
161161
}
162-
} catch (e: Throwable) {
162+
} catch (e: RuntimeException) {
163+
// OEM View subclasses have been observed throwing during hierarchy traversal
164+
// (e.g. Redmi's TextView NPE). Release the frame gate so a single bad frame
165+
// doesn't wedge the recorder. Errors (OOM, LinkageError) intentionally propagate.
163166
options.logger.log(WARNING, "Failed to process replay frame", e)
164167
finishFrame()
165168
}
@@ -397,8 +400,9 @@ internal class PixelCopyStrategy(
397400
maskRenderer.close()
398401
},
399402
)
400-
// close() typically runs after ReplayIntegration has shut down the executor, so submit may
401-
// return null. Fall back to running cleanup inline so the bitmap + mask renderer are freed.
403+
// ReplayExecutorService.submit returns null only on genuine rejection (post-shutdown);
404+
// inline execution on the worker thread returns a completed future. Fall back to running
405+
// cleanup here so the bitmap + mask renderer are freed even when the executor is dead.
402406
if (executor.submit(cleanup) == null) {
403407
cleanup.run()
404408
}

sentry-android-replay/src/main/java/io/sentry/android/replay/util/ReplayExecutorService.kt

Lines changed: 25 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import io.sentry.SentryLevel.ERROR
44
import io.sentry.SentryOptions
55
import java.util.concurrent.Future
66
import java.util.concurrent.ScheduledExecutorService
7+
import java.util.concurrent.TimeUnit
78
import java.util.concurrent.TimeUnit.MILLISECONDS
89

910
/**
@@ -14,11 +15,20 @@ internal class ReplayExecutorService(
1415
private val delegate: ScheduledExecutorService,
1516
private val options: SentryOptions,
1617
) : ScheduledExecutorService by delegate {
18+
/**
19+
* Submits [task] for execution and returns a [Future] describing what happened. The return value
20+
* has three distinct outcomes callers can rely on:
21+
* - [CompletedFuture] — the caller is already on the replay worker thread, so the task was run
22+
* inline before this method returned. Skips the queue.
23+
* - A regular [Future] from the underlying [ScheduledExecutorService] — the task was queued and
24+
* will run asynchronously.
25+
* - `null` — the underlying executor rejected the submission (typically because it has been shut
26+
* down). The task did NOT run; callers that need cleanup must handle it themselves.
27+
*/
1728
override fun submit(task: Runnable): Future<*>? {
1829
if (Thread.currentThread().name.startsWith("SentryReplayIntegration")) {
19-
// we're already on the worker thread, no need to submit
2030
task.run()
21-
return null
31+
return CompletedFuture
2232
}
2333
return try {
2434
delegate.submit {
@@ -68,3 +78,16 @@ internal class ReplayExecutorService(
6878
}
6979

7080
internal class ReplayRunnable(val taskName: String, delegate: Runnable) : Runnable by delegate
81+
82+
/** A Future that represents an already-completed inline execution — never used as null. */
83+
internal object CompletedFuture : Future<Unit> {
84+
override fun cancel(mayInterruptIfRunning: Boolean): Boolean = false
85+
86+
override fun isCancelled(): Boolean = false
87+
88+
override fun isDone(): Boolean = true
89+
90+
override fun get() {}
91+
92+
override fun get(timeout: Long, unit: TimeUnit) {}
93+
}

0 commit comments

Comments
 (0)