Skip to content

Free native resources on three rare decoder error paths#2587

Open
iurisilvio wants to merge 1 commit into
Automattic:masterfrom
iurisilvio:fix/decoder-error-path-leaks
Open

Free native resources on three rare decoder error paths#2587
iurisilvio wants to merge 1 commit into
Automattic:masterfrom
iurisilvio:fix/decoder-error-path-leaks

Conversation

@iurisilvio
Copy link
Copy Markdown

Fixes #2586.

Three small ownership bugs on rare error paths in image decoders and pattern construction. None are reachable in normal operation, but each is a real leak when its gate fires.

Changes

  • Image::loadGIFFromBuffer: delete[] the decode buffer before returning CAIRO_STATUS_READ_ERROR when neither a local nor a global color map exists. Per-occurrence leak is the full width * height * 4 buffer.
  • Image::assignDataAsMime: capture the cairo_surface_set_mime_data return status; on non-success, reverse the Napi::MemoryManagement::AdjustExternalMemory(env, len) and free both mime_data (a copy of the JPEG buffer) and mime_closure. Cairo doesn't take ownership in the failure case and never calls clearMimeData, so both allocations otherwise live forever.
  • create_transparent_pattern: destroy mask_context and mask_surface before returning NULL when cairo_create reports an error status. Only reachable on cairo OOM.

Verification

These are behind error/OOM gates that aren't reachable from JS without poking at internals, so no RSS reproduction is attached. The fixes are visible by inspection and total +12 / -1 lines.

- Image::loadGIFFromBuffer: delete[] the decode buffer before returning
  CAIRO_STATUS_READ_ERROR when neither a local nor a global color map
  exists. The full width*height*4 buffer was previously dropped.

- Image::assignDataAsMime: capture the cairo_surface_set_mime_data return
  status; on non-success, reverse the AdjustExternalMemory(env, len) and
  free both mime_data (a copy of the JPEG buffer) and mime_closure, which
  cairo never takes ownership of in the failure case.

- create_transparent_pattern: destroy mask_context and mask_surface
  before returning NULL when cairo_create reports an error status (cairo
  OOM).

All three are real ownership bugs but sit behind error/OOM gates that
aren't reachable in normal operation, so there's no measurable RSS slope
to attach. Bundled because each is a one- or two-line fix in the same
family.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Three rare error paths in image decoders leak native resources

1 participant