[pull] main from getsentry:main - #404
Merged
Merged
Conversation
* fix(replay): Prevent concurrent PixelCopy frame access Keep PixelCopy, masking, compositing, and cleanup from accessing the shared bitmap concurrently. Fixes GH-5340 Co-Authored-By: Codex <noreply@openai.com> * changelog * fix(android-replay): Preserve pending PixelCopy capture Re-arm the recorder's content-change gate when a capture is skipped because another frame is still in flight. This ensures the latest UI state is retried on the next capture interval. Refs GH-5340 Co-Authored-By: OpenAI Codex <noreply@openai.com> * fix(replay): Release PixelCopy frame gate and cleanup on failure paths Two bugbot findings on #5808: - Frame gate stuck on callback errors: viewhierarchy traversal or captureSurfaceViews throwing between the PixelCopy success check and the executor submit left frameInFlight = true forever, silently wedging all future captures. Wrap the post-success block in a try/finally that releases the gate unless work was successfully handed off. - Cleanup lost after executor shutdown: close() typically runs after ReplayIntegration has shut down the replay executor, so submit() returns null and the bitmap + maskRenderer were never released. Fall back to running cleanup inline in that case. * chore(replay): Drop redundant handedOff flag in capture callback The mask branch already knows via 'submitted == null' whether the executor took ownership; the surface-view branch always hands off. Flatten to explicit finishFrame() calls on the two failure paths (mask null-submit, outer catch) instead of tracking handoff state across a try/finally. * 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. * chore(replay): Drop redundant cleanupScheduled guard Cleanup body is already idempotent (screenshot.isRecycled check, MaskRenderer.close guards on isInitialized + isRecycled). A stray extra scheduleCleanup would just submit a no-op — not worth the AtomicBoolean. * test(replay): Fix recursive close and assert masking is skipped on close race The mock executor closed the strategy on every submit(); since close() itself submits the cleanup task, this recursed close() -> scheduleCleanup() -> submit() until the stack overflowed. Close only when the mask task is submitted. The prior "does not crash" assertion was also vacuous under the frameInFlight gate (passed even with the isClosed guard removed). Assert instead that no screenshot is emitted once close() races masking, which fails if the guard in applyMaskingAndNotify is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(replay): Guard finishFrame cleanup with CAS to avoid recycling a bitmap a new capture is using finishFrame cleared frameInFlight before checking isClosed, so a new capture could take the gate and start PixelCopy into the shared screenshot while the old finishFrame went on to scheduleCleanup after close() flipped isClosed — recycling the bitmap mid-write. Re-take the gate with compareAndSet before cleaning up so the losing frame backs off. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(replay): Make close's idle cleanup claim the gate atomically close() checked !frameInFlight.get() non-atomically before scheduleCleanup, so a capture racing in after the check could take the gate, see isClosed, and have its finishFrame schedule a second cleanup. Extract the shared "claim the gate, then the winner cleans up once" invariant into cleanUpIfIdle() so close() and finishFrame() use the same CAS. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(replay): Return CompletedFuture from inlineExecutor to match ReplayExecutorService inlineExecutor() returned null from submit, which under the executor's contract means "rejected" — making capture()'s null-fallback finishFrame run on top of the mask task's own finally, a double-release production never hits on the inline path. Return CompletedFuture so the fixture matches the real inline semantics. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(replay): Hold frame gate across emitLastScreenshot to prevent concurrent bitmap access emitLastScreenshot runs on the main thread, so the downstream consumer queues its bitmap read (JPEG compress) to the executor asynchronously. Without the gate, the next capture tick's PixelCopy.request writes into the shared screenshot while the queued read is still in flight. Submit the consumer call to the executor so it runs inline on the worker thread while the gate is held, same pattern as the masked capture path. Also fixes the close-race test mock to return CompletedFuture instead of null, matching the executor contract (same fix as inlineExecutor in cd3a7c0). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Codex <noreply@openai.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )