PANA-9097: Copy bitmaps before reading them - #1440
jonathanmos wants to merge 1 commit into
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: de4f2cd432
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
de4f2cd to
d2765cc
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One or more issues must be addressed before approval.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR hardens Android Session Replay image capture by copying bitmap-backed drawables before background processing and returning null when copying fails.
Changes:
- Adds safe bitmap copy/scale helpers.
- Updates drawable extraction and copying to avoid shared bitmap references.
- Adds unit tests for successful and failed copy paths.
| File | Description |
|---|---|
| packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/resources/ReactDrawableCopierTest.kt | Updated as part of this pull request. |
| packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/extensions/ReactDrawablesExtTest.kt | Updated as part of this pull request. |
| packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/resources/ReactDrawableCopier.kt | Updated as part of this pull request. |
| packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/extensions/ReactDrawablesExt.kt | Updated as part of this pull request. |
| packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/extensions/BitmapExt.kt | Updated as part of this pull request. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
d2765cc to
f9eaae1
Compare
…concurrent recycle
f9eaae1 to
458ad2d
Compare
What does this PR do?
Session Replay's image-capture code (ReactDrawableCopier, ReactDrawablesExt) could hand out a live Bitmap reference instead of copying it, on paths where a BitmapDrawable's pixels get read on a background thread. If the bitmap's owner (e.g. Fresco) recycles it concurrently, native pixel memory is freed before isRecycled() flips — so a later native read aborts the process with an uncatchable SIGABRT.
Bitmap extraction now always produces a private copy (Bitmap.safeCopy()/safeScale()) before further processing, and ReactDrawableCopier no longer routes BitmapDrawables through the aliasing DefaultDrawableCopier. On copy/scale failure (bitmap already recycled), the code now returns null and lets the existing, already-tested ResourceResolver fallback degrade gracefully (empty ImageWireframe) instead of fabricating a blank bitmap.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)