Skip to content

fix!: manage external buffer lifetime - #66

Open
GrapeBaBa wants to merge 5 commits into
mainfrom
gr/external-buffer-lifecycle
Open

fix!: manage external buffer lifetime#66
GrapeBaBa wants to merge 5 commits into
mainfrom
gr/external-buffer-lifecycle

Conversation

@GrapeBaBa

@GrapeBaBa GrapeBaBa commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Motivation

The .external_buffer return hint passed borrowed Zig slices to JavaScript without copying or registering a finalizer. JavaScript can retain the Buffer after the native callback returns, leaving it backed by stack, arena, or otherwise freed memory. External buffers should retain their zero-copy behavior while making ownership and cleanup explicit.

Description

  • Add an allocator-backed OwnedBuffer that transfers its allocation to JavaScript.
  • Require .external_buffer callbacks to return OwnedBuffer; ordinary byte-slice returns continue to copy.
  • Release the backing allocation and finalizer context from the N-API finalizer.
  • Match napi-rs ownership semantics: conversion consumes OwnedBuffer; environments that disallow external buffers copy the bytes and release the original allocation, while other errors preserve ownership already accepted by the N-API finalizer.
  • Use a normal Buffer for empty data and document the allocator lifetime contract.
  • Test the public copy contract and finalizer-context allocation failure, plus a real Node --expose-gc integration test that verifies the external data and finalizer context remain live while reachable and are both released after GC.

@GrapeBaBa
GrapeBaBa marked this pull request as ready for review July 28, 2026 13:04
@nazarhussain

Copy link
Copy Markdown
Contributor

This has good test coverage for that tricky case. @GrapeBaBa Good work 👍🏼

@matthewkeil matthewkeil moved this from In Progress to Review Ready/Requested in Lodestar Team Coordination Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Review Ready/Requested

Development

Successfully merging this pull request may close these issues.

3 participants