Release `Buffer` and `ArrayBuffer` instances that were created through
our addon APIs and have finalizers attached to them only after V8 has
called the deleter callback passed to the `BackingStore`, instead of
relying on our own GC callback(s).
This fixes the following race condition:
1. Addon code allocates pointer P via `malloc`.
2. P is passed into `napi_create_external_buffer` with a finalization
callback which calls `free(P)`. P is inserted into V8’s global array
buffer table for tracking.
3. The finalization callback is executed on GC. P is freed and returned
to the allocator. P is not yet removed from V8’s global array
buffer table. (!)
4. Addon code attempts to allocate memory once again. The allocator
returns P, as it is now available.
5. P is passed into `napi_create_external_buffer`. P still has not been
removed from the v8 global array buffer table.
6. The world ends with `Check failed: result.second`.
Since our API contract is to call the finalizer on the JS thread on
which the `ArrayBuffer` was created, but V8 may call the `BackingStore`
deleter callback on another thread, fixing this requires posting
a task back to the JS thread.
Refs: https://github.com/nodejs/node/issues/32463#issuecomment-625877175
Fixes: https://github.com/nodejs/node/issues/32463
PR-URL: https://github.com/nodejs/node/pull/33321
Reviewed-By: James M Snell <jasnell@gmail.com>
Currently, there are a number of compiler warnings generated when
building the addons on Linux, for example:
make[1]: Entering directory '/node/test/addons/zlib-binding/build'
CXX(target) Release/obj.target/binding/binding.o
SOLINK_MODULE(target) Release/obj.target/binding.node
COPY Release/binding.node
make[1]: Leaving directory '/node/test/addons/zlib-binding/build'
In file included from ../binding.cc:1:
/node/src/node.h:515:51: warning:
cast between incompatible function types from
'void (*)(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>)' to
'node::addon_context_register_func' {aka
'void (*)(v8::Local<v8::Object>,
v8::Local<v8::Value>,
v8::Local<v8::Context>,
void*)'} [-Wcast-function-type]
(node::addon_context_register_func) (regfunc), \
^
/node/src/node.h:533:3:
note: in expansion of macro 'NODE_MODULE_CONTEXT_AWARE_X'
NODE_MODULE_CONTEXT_AWARE_X(modname, regfunc, NULL, 0)
^~~~~~~~~~~~~~~~~~~~~~~~~~~
../binding.cc:58:1:
note: in expansion of macro 'NODE_MODULE_CONTEXT_AWARE'
NODE_MODULE_CONTEXT_AWARE(NODE_GYP_MODULE_NAME, Initialize)
^~~~~~~~~~~~~~~~~~~~~~~~~
This commit adds the flag -Wno-cast-function-type to suppress these
warnings. With this change the warnings are not displayed anymore and
the output matches that of osx when running
'make -j8 test/addons/.buildstamp'.
PR-URL: https://github.com/nodejs/node/pull/25663
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Currently the nm_modname does not match the file name of the resulting
module. In fact, the nm_modname is pretty arbitrary. This seeks to
introduce some consistency into the nm_modname property by having the
name of the module appear in exactly one place: the "target_name"
property of the gyp target that builds the module.
PR-URL: https://github.com/nodejs/node/pull/15209
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Having semicolons there runs counter to our documentation and illicits
warnings in pedantic mode. This removes semicolons from after uses of
NODE_MODULE and NODE_MODULE_CONTEXT_AWARE_BUILTIN.
PR-URL: https://github.com/nodejs/node/pull/12919
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Alexey Orlenko <eaglexrlnk@gmail.com>
The doc/api/addons.md document contains examples of Addon
Initialization functions that take a parameter named exports.
This also matches the name used in node.cc when calling:
mp->nm_register_func(exports, module, mp->nm_priv);
Currently, a number of the tests name this same parameter target. This
commit renames target to exports for consistency.
PR-URL: https://github.com/nodejs/node/pull/9135
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Currently when running configure with the --debug option in combination
with the tests (./configure --debug && make -j8 test) there are a few
addon tests that fail with error messages similar to this:
=== release test ===
Path: addons/load-long-path/test
fs.js:558
return binding.open(pathModule._makeLong(path), stringToFlags(flags),
mode);
^
Error: ENOENT: no such file or directory, open
'/nodejs/node/test/addons/load-long-path/build/Release/binding.node'
at Object.fs.openSync (fs.js:558:18)
at Object.fs.readFileSync (fs.js:468:33)
at Object.<anonymous>
(/nodejs/node/test/addons/load-long-path/test.js:28:19)
at Module._compile (module.js:560:32)
at Object.Module._extensions..js (module.js:569:10)
at Module.load (module.js:477:32)
at tryModuleLoad (module.js:436:12)
at Function.Module._load (module.js:428:3)
at Module.runMain (module.js:594:10)
at run (bootstrap_node.js:382:7)
Command: out/Release/node
/nodejs/node/test/addons/load-long-path/test.js
This commit allows for the tests to pass even if the configured build
type is of type debug.
PR-URL: https://github.com/nodejs/node/pull/8836
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
There is no real need and it causes endless grief on Windows with some
of the upcoming changes.
PR-URL: https://github.com/nodejs/node/pull/6734
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
For consistency with the newly added src/base64.h header, check that
NODE_WANT_INTERNALS is defined and set in internal headers.
PR-URL: https://github.com/nodejs/node/pull/6948
Refs: https://github.com/nodejs/node/pull/6910
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Neuter external `nullptr` buffers, otherwise their contents will be
materialized on access, and the buffer instance will be internalized.
This leads to a crash like this:
v8::ArrayBuffer::Neuter Only externalized ArrayBuffers can be
neutered
Fix: #3619
PR-URL: https://github.com/nodejs/node/pull/3624
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Trevor Norris <trev.norris@gmail.com>
