Skip to content

Commit cdeb79a

Browse files
romtsncodex
andcommitted
ref(android): Confine replay lifecycle to main thread
Serialize replay lifecycle mutations on Android's main thread and keep replay cache cleanup ordered on the replay executor. Remove locks that could block lifecycle callbacks while preserving shutdown ordering. Refs JAVA-665 Co-Authored-By: OpenAI Codex <noreply@openai.com>
1 parent 32811a3 commit cdeb79a

11 files changed

Lines changed: 306 additions & 358 deletions

File tree

sentry-android-replay/src/main/java/io/sentry/android/replay/ReplayCache.kt

Lines changed: 27 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ import java.io.File
2222
import java.io.StringReader
2323
import java.util.Date
2424
import java.util.LinkedList
25-
import java.util.concurrent.TimeUnit.MILLISECONDS
2625
import java.util.concurrent.atomic.AtomicBoolean
2726

2827
/**
@@ -41,7 +40,6 @@ import java.util.concurrent.atomic.AtomicBoolean
4140
public class ReplayCache(private val options: SentryOptions, private val replayId: SentryId) :
4241
Closeable {
4342
private val isClosed = AtomicBoolean(false)
44-
private val encoderLock = AutoClosableReentrantLock()
4543
private val lock = AutoClosableReentrantLock()
4644
private val framesLock = AutoClosableReentrantLock()
4745
private var encoder: SimpleVideoEncoder? = null
@@ -152,28 +150,26 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
152150
}
153151

154152
encoder =
155-
encoderLock.acquire().use {
156-
SimpleVideoEncoder(
157-
options,
158-
MuxerConfig(
159-
file = videoFile,
160-
recordingHeight = height,
161-
recordingWidth = width,
162-
frameRate = frameRate,
163-
bitRate = bitRate,
164-
),
165-
)
166-
.apply {
167-
// the constructor already opened the MediaMuxer, so release it if start() fails,
168-
// otherwise the encoder is never assigned and its resources leak (CloseGuard warning)
169-
try {
170-
start()
171-
} catch (t: Throwable) {
172-
release()
173-
throw t
174-
}
153+
SimpleVideoEncoder(
154+
options,
155+
MuxerConfig(
156+
file = videoFile,
157+
recordingHeight = height,
158+
recordingWidth = width,
159+
frameRate = frameRate,
160+
bitRate = bitRate,
161+
),
162+
)
163+
.apply {
164+
// the constructor already opened the MediaMuxer, so release it if start() fails,
165+
// otherwise the encoder is never assigned and its resources leak (CloseGuard warning)
166+
try {
167+
start()
168+
} catch (t: Throwable) {
169+
release()
170+
throw t
175171
}
176-
}
172+
}
177173

178174
val step = 1000 / frameRate.toLong()
179175
var frameCount = 0
@@ -209,20 +205,15 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
209205

210206
if (frameCount == 0) {
211207
options.logger.log(DEBUG, "Generated a video with no frames, not capturing a replay segment")
212-
encoderLock.acquire().use {
213-
encoder?.release()
214-
encoder = null
215-
}
208+
encoder?.release()
209+
encoder = null
216210
deleteFile(videoFile)
217211
return null
218212
}
219213

220-
var videoDuration: Long
221-
encoderLock.acquire().use {
222-
encoder?.release()
223-
videoDuration = encoder?.duration ?: 0
224-
encoder = null
225-
}
214+
encoder?.release()
215+
val videoDuration = encoder?.duration ?: 0
216+
encoder = null
226217

227218
rotate(until = (from + duration))
228219

@@ -235,7 +226,7 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
235226
}
236227
return try {
237228
val bitmap = BitmapFactory.decodeFile(frame.screenshot.absolutePath)
238-
encoderLock.acquire().use { encoder?.encode(bitmap) }
229+
encoder?.encode(bitmap)
239230
bitmap.recycle()
240231
true
241232
} catch (e: Throwable) {
@@ -281,27 +272,10 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
281272
}
282273

283274
override fun close() {
284-
// close() is called inline from the lifecycle path (ReplayIntegration.stop/close), which holds
285-
// its own lock, so blocking here can freeze the main thread. If the encoder is wedged in a
286-
// native MediaCodec call we'd never get the lock, so we give up instead: the already-dead codec
287-
// is not released (leaking a native handle), which beats an ANR.
288275
try {
289-
val token = encoderLock.tryAcquire(ENCODER_RELEASE_TIMEOUT_MS, MILLISECONDS)
290-
if (token == null) {
291-
options.logger.log(
292-
WARNING,
293-
"Timed out waiting for the video encoder, skipping its release to not block the caller",
294-
)
295-
} else {
296-
token.use {
297-
encoder?.release()
298-
encoder = null
299-
}
300-
}
301-
} catch (e: InterruptedException) {
302-
Thread.currentThread().interrupt()
276+
encoder?.release()
277+
encoder = null
303278
} finally {
304-
// has to happen on all paths, callers rely on it to stop persisting segment values
305279
isClosed.set(true)
306280
}
307281
}
@@ -333,13 +307,6 @@ public class ReplayCache(private val options: SentryOptions, private val replayI
333307
}
334308

335309
internal companion object {
336-
/**
337-
* How long [close] waits for the video encoder to become available. Below Android's ~5s ANR
338-
* budget, and above the encoder's own bail-out (see MAX_EOS_STALL_ITERATIONS), so an encoder
339-
* that's merely slow is still awaited rather than abandoned.
340-
*/
341-
private const val ENCODER_RELEASE_TIMEOUT_MS = 2000L
342-
343310
internal const val ONGOING_SEGMENT = ".ongoing_segment"
344311

345312
internal const val SEGMENT_KEY_HEIGHT = "config.height"

0 commit comments

Comments
 (0)