Repository navigation
External data lifetime for Buffer and ArrayBuffer #258
Description
While writing up the documentation for Buffer and ArrayBuffer I came across some suspicious looking factory methods:
static ArrayBuffer New(napi_env env, void* externalData, size_t byteLength);
static Buffer<T> New(napi_env env, T* data, size_t length);Digging through the code a bit, it appears that these methods take in a pointer to data and then create an external wrapper object with no finalizer.
Looking at the NAN documentation for buffers it seems like the behavior is to take ownership of the memory and then later free it. Even NAN's approach is arguably dangerous since it makes assumptions about how and where the memory was allocated.
Since node-addon-api doesn't actually free the data and doesn't provide any way for the user to free the data, the only use case I can see for this API is for data which has a static lifetime.
Given their limited (and dangerous) usage, maybe we should consider removing these overloads?
Activity
kfarnung commentedon May 3, 2018
Per discussion today investigate mimicking the NAN behavior.
github-actions commentedon Dec 18, 2020
This issue is stale because it has been open many days with no activity. It will be closed soon unless the stale label is removed or a comment is made.
mhdawson commentedon Jan 15, 2021
@NickNaso will review and lead the discussion in a future team meeting.
mhdawson commentedon Feb 12, 2021
Touched on in the meeting today, no update, but @NickNaso still looking at it.
NickNaso commentedon Feb 26, 2021
Hi everybody, I did not have the time to work on issue until today. I'm starting work on this now, I will update at next Node-API meeting.
gabrielschulhof commentedon Mar 5, 2021
@NickNaso will modify the
BufferandArrayBufferdocumentation to add a warning for the::New()variants that do not accept a finalizer about the fact thatBuffers andArrayBuffers created in such a way will not provide the caller with an opportunity to free the data when they get garbage-collected.