Skip to content

Android: fix Bitmap memory leaks in CameraCaptureManager.snap#41902

Merged
obviyus merged 2 commits intoopenclaw:mainfrom
Kaneki-x:fix/android-camera-snap-bitmap-leak
Mar 22, 2026
Merged

Android: fix Bitmap memory leaks in CameraCaptureManager.snap#41902
obviyus merged 2 commits intoopenclaw:mainfrom
Kaneki-x:fix/android-camera-snap-bitmap-leak

Conversation

@Kaneki-x
Copy link
Contributor

Summary

CameraCaptureManager.snap() leaks native Bitmap memory on every camera snap call. After capturing, rotating, and scaling the image, the intermediate rotated and final scaled Bitmaps are never recycled:

  • When scaling occurs, rotated.scale() creates a new Bitmap but the original rotated is leaked.
  • After JpegSizeLimiter.compressToLimit returns, the scaled Bitmap is never recycled.
  • Each snap can leak 1–2 full-resolution Bitmaps worth of native memory.

Fix

  • Recycle rotated immediately after scale() produces a different Bitmap.
  • Wrap the compression + payload creation in try/finally to ensure scaled.recycle() is always called.

The encode lambda inside compressToLimit already handles its own intermediate Bitmaps correctly (recycling when bitmap !== scaled); this fix only addresses the outer scope leaks.

Same pattern as #41888 (PhotosHandler) and #41889 (CanvasController).

Test plan

  • Code review: every Bitmap allocation path in snap() now has a matching recycle().
  • Verified rotateBitmapByExif already recycles the input when creating a new rotated Bitmap, so no double-recycle.
  • Verified the encode lambda's identity check (bitmap !== scaled) remains correct after the fix.
  • Manual test: take multiple photos via the Android app and confirm no OOM or increasing native memory.

This PR was authored with AI assistance.

Made with Cursor

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Mar 10, 2026

Greptile Summary

This PR fixes two native Bitmap memory leaks in CameraCaptureManager.snap():

  1. rotated leak after scalingrotated.scale() now assigns its result to a local variable s, and rotated.recycle() is called when s !== rotated (i.e., a new Bitmap was actually allocated). This correctly handles the degenerate case where Bitmap.scale() returns the same instance.

  2. scaled leak after compression — the compression and payload-creation block is wrapped in try/finally, so scaled.recycle() is guaranteed to be called regardless of whether JpegSizeLimiter.compressToLimit succeeds, throws an encoding failure, or throws an exception.

The implementation is correct:

  • JpegSizeLimiter.compressToLimit is fully synchronous, so scaled is never accessed after finally runs.
  • The identity check (s !== rotated) before recycling rotated correctly prevents double-recycling.
  • The Payload returned from the try block holds only a JSON String (no Bitmap reference), so recycling scaled in finally is safe.
  • The existing per-iteration intra-lambda recycling logic for intermediate downscaled Bitmaps (the bitmap !== scaled checks) is preserved and correct.

Confidence Score: 5/5

  • This PR is safe to merge. The Bitmap lifecycle fixes are minimal, well-reasoned, and correctly implemented with proper identity checks to prevent double-recycling.
  • The changes are narrowly scoped to two missing recycle() calls in the snap() method. The identity-guarded recycle of rotated (line 125) and the try/finally around scaled (lines 131-165) correctly cover all code paths: normal completion, JPEG encoding failure, and exception scenarios. All intermediate Bitmaps created by the encode lambda are properly recycled. No logic changes were made to the compression algorithm or payload structure. The implementation has been verified against the actual code.
  • No files require special attention

Last reviewed commit: f4cc719

@Kaneki-x
Copy link
Contributor Author

@steipete @obviyus Would appreciate a review when you have a chance — this fixes Bitmap memory leaks in CameraCaptureManager.snap (same pattern as #41888).

obviyus added a commit to Kaneki-x/openclaw that referenced this pull request Mar 22, 2026
@obviyus obviyus force-pushed the fix/android-camera-snap-bitmap-leak branch from f4cc719 to a232193 Compare March 22, 2026 03:11
@obviyus obviyus force-pushed the fix/android-camera-snap-bitmap-leak branch from a232193 to 4c002a1 Compare March 22, 2026 03:13
@obviyus obviyus merged commit eea84bc into openclaw:main Mar 22, 2026
10 checks passed
@obviyus
Copy link
Contributor

obviyus commented Mar 22, 2026

Landed on main.

Thanks @Kaneki-x.

@aisle-research-bot
Copy link

aisle-research-bot bot commented Mar 22, 2026

🔒 Aisle Security Analysis

We found 1 potential security issue(s) in this PR:

# Severity Title
1 🟡 Medium Unbounded memory allocation during JPEG compression retries (camera.snap) can crash app

1. 🟡 Unbounded memory allocation during JPEG compression retries (camera.snap) can crash app

Property Value
Severity Medium
CWE CWE-400
Location apps/android/app/src/main/java/ai/openclaw/app/node/CameraCaptureManager.kt:141-157

Description

The camera.snap path attempts to enforce a 5MB payload limit, but the encoding loop allocates the full JPEG in memory before the size check and duplicates it via toByteArray() on every attempt.

  • paramsJson influences processing (e.g., quality, maxWidth), and can cause the code to operate on larger bitmaps and run multiple encode attempts.
  • Each encode attempt creates a new ByteArrayOutputStream, lets Bitmap.compress() write the entire JPEG (which may be far larger than maxBytes), then calls out.toByteArray() which copies the internal buffer into a new ByteArray.
  • JpegSizeLimiter.compressToLimit() can call encode() up to 37 times (1 + 6×6). On high-resolution frames this can cause large transient allocations and CPU work, potentially triggering OOM / ANR (Denial of Service) even though maxBytes is intended to cap output.

Vulnerable code:

val out = ByteArrayOutputStream()
if (!bitmap.compress(Bitmap.CompressFormat.JPEG, q, out)) {
  ...
}
out.toByteArray()

Recommendation

Prevent unbounded buffering during encode attempts.

Option A (recommended): use a size-limiting OutputStream so compression aborts once the limit is exceeded, avoiding huge buffers and the toByteArray() copy on oversized results.

Example:

class TooLargeException: RuntimeException()

class MaxSizeOutputStream(
  private val delegate: java.io.OutputStream,
  private val maxBytes: Int,
): java.io.OutputStream() {
  private var count = 0
  override fun write(b: Int) {
    if (count + 1 > maxBytes) throw TooLargeException()
    delegate.write(b)
    count += 1
  }
  override fun write(b: ByteArray, off: Int, len: Int) {
    if (count + len > maxBytes) throw TooLargeException()
    delegate.write(b, off, len)
    count += len
  }
}

​// inside encode:
val out = ByteArrayOutputStream()
val limited = MaxSizeOutputStream(out, maxBytes + 1)
val ok = try {
  bitmap.compress(Bitmap.CompressFormat.JPEG, q, limited)
} catch (_: TooLargeException) {
  true // treat as "encoded but too large" and let the limiter reduce quality/scale
}
if (!ok) error("UNAVAILABLE: failed to encode JPEG")
return out.toByteArray()

Then adapt JpegSizeLimiter / the encode lambda contract so “too large” results don’t become fatal encode failures.

Option B: enforce a hard upper bound on maxWidth (and/or always downscale to a safe maximum) so high-resolution inputs can’t trigger extreme work.

Also consider moving encoding off the main thread to reduce ANR risk.


Analyzed PR: #41902 at commit 4c002a1

Last updated on: 2026-03-22T03:38:32Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants