diff --git a/doc/array_buffer.md b/doc/array_buffer.md index 0c12614f3..3be6b42a2 100644 --- a/doc/array_buffer.md +++ b/doc/array_buffer.md @@ -23,6 +23,9 @@ Returns a new `Napi::ArrayBuffer` instance. ### New +> When `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` is defined, this method is not available. +> See [External Buffer][] for more information. + Wraps the provided external data into a new `Napi::ArrayBuffer` instance. The `Napi::ArrayBuffer` instance does not assume ownership for the data and @@ -48,6 +51,9 @@ Returns a new `Napi::ArrayBuffer` instance. ### New +> When `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` is defined, this method is not available. +> See [External Buffer][] for more information. + Wraps the provided external data into a new `Napi::ArrayBuffer` instance. The `Napi::ArrayBuffer` instance does not assume ownership for the data and @@ -74,6 +80,9 @@ Returns a new `Napi::ArrayBuffer` instance. ### New +> When `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` is defined, this method is not available. +> See [External Buffer][] for more information. + Wraps the provided external data into a new `Napi::ArrayBuffer` instance. The `Napi::ArrayBuffer` instance does not assume ownership for the data and expects it @@ -153,3 +162,4 @@ bool Napi::ArrayBuffer::IsDetached() const; Returns `true` if this `ArrayBuffer` has been detached. [`Napi::Object`]: ./object.md +[External Buffer]: ./external_buffer.md diff --git a/doc/buffer.md b/doc/buffer.md index 1b6a3e1d6..427eeee2f 100644 --- a/doc/buffer.md +++ b/doc/buffer.md @@ -22,6 +22,9 @@ Returns a new `Napi::Buffer` object. ### New +> When `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` is defined, this method is not available. +> See [External Buffer][] for more information. + Wraps the provided external data into a new `Napi::Buffer` object. The `Napi::Buffer` object does not assume ownership for the data and expects it to be @@ -47,6 +50,9 @@ Returns a new `Napi::Buffer` object. ### New +> When `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` is defined, this method is not available. +> See [External Buffer][] for more information. + Wraps the provided external data into a new `Napi::Buffer` object. The `Napi::Buffer` object does not assume ownership for the data and expects it @@ -72,6 +78,9 @@ Returns a new `Napi::Buffer` object. ### New +> When `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` is defined, this method is not available. +> See [External Buffer][] for more information. + Wraps the provided external data into a new `Napi::Buffer` object. The `Napi::Buffer` object does not assume ownership for the data and expects it to be @@ -98,6 +107,93 @@ static Napi::Buffer Napi::Buffer::New(napi_env env, Returns a new `Napi::Buffer` object. +### NewOrCopy + +Wraps the provided external data into a new `Napi::Buffer` object. When the +[external buffer][] is not supported, allocates a new `Napi::Buffer` object and +copies the provided external data into it. + +The `Napi::Buffer` object does not assume ownership for the data and expects it to be +valid for the lifetime of the object. Since the `Napi::Buffer` is subject to garbage +collection this overload is only suitable for data which is static and never +needs to be freed. + +This factory method will not provide the caller with an opportunity to free the +data when the `Napi::Buffer` gets garbage-collected. If you need to free the +data retained by the `Napi::Buffer` object please use other variants of the +`Napi::Buffer::New` factory method that accept `Napi::Finalizer`, which is a +function that will be invoked when the `Napi::Buffer` object has been +destroyed. + +```cpp +static Napi::Buffer Napi::Buffer::NewOrCopy(napi_env env, T* data, size_t length); +``` + +- `[in] env`: The environment in which to create the `Napi::Buffer` object. +- `[in] data`: The pointer to the external data to expose. +- `[in] length`: The number of `T` elements in the external data. + +Returns a new `Napi::Buffer` object. + +### NewOrCopy + +Wraps the provided external data into a new `Napi::Buffer` object. When the +[external buffer][] is not supported, allocates a new `Napi::Buffer` object and +copies the provided external data into it and the `finalizeCallback` is invoked +immediately. + +The `Napi::Buffer` object does not assume ownership for the data and expects it +to be valid for the lifetime of the object. The data can only be freed once the +`finalizeCallback` is invoked to indicate that the `Napi::Buffer` has been released. + +```cpp +template +static Napi::Buffer Napi::Buffer::NewOrCopy(napi_env env, + T* data, + size_t length, + Finalizer finalizeCallback); +``` + +- `[in] env`: The environment in which to create the `Napi::Buffer` object. +- `[in] data`: The pointer to the external data to expose. +- `[in] length`: The number of `T` elements in the external data. +- `[in] finalizeCallback`: The function to be called when the `Napi::Buffer` is + destroyed. It must implement `operator()`, accept an Napi::Env, a `T*` (which is the + external data pointer), and return `void`. + +Returns a new `Napi::Buffer` object. + +### NewOrCopy + +Wraps the provided external data into a new `Napi::Buffer` object. When the +[external buffer][] is not supported, allocates a new `Napi::Buffer` object and +copies the provided external data into it and the `finalizeCallback` is invoked +immediately. + +The `Napi::Buffer` object does not assume ownership for the data and expects it to be +valid for the lifetime of the object. The data can only be freed once the +`finalizeCallback` is invoked to indicate that the `Napi::Buffer` has been released. + +```cpp +template +static Napi::Buffer Napi::Buffer::NewOrCopy(napi_env env, + T* data, + size_t length, + Finalizer finalizeCallback, + Hint* finalizeHint); +``` + +- `[in] env`: The environment in which to create the `Napi::Buffer` object. +- `[in] data`: The pointer to the external data to expose. +- `[in] length`: The number of `T` elements in the external data. +- `[in] finalizeCallback`: The function to be called when the `Napi::Buffer` is + destroyed. It must implement `operator()`, accept an Napi::Env, a `T*` (which is the + external data pointer) and `Hint*`, and return `void`. +- `[in] finalizeHint`: The hint to be passed as the second parameter of the + finalize callback. + +Returns a new `Napi::Buffer` object. + ### Copy Allocates a new `Napi::Buffer` object and copies the provided external data into it. @@ -148,3 +244,4 @@ size_t Napi::Buffer::Length() const; Returns the number of `T` elements in the external data. [`Napi::Uint8Array`]: ./typed_array_of.md +[External Buffer]: ./external_buffer.md diff --git a/doc/external_buffer.md b/doc/external_buffer.md new file mode 100644 index 000000000..25942436a --- /dev/null +++ b/doc/external_buffer.md @@ -0,0 +1,18 @@ +# External Buffer + +**Some runtimes other than Node.js have dropped support for external buffers**. +On runtimes other than Node.js, node-api methods may return +`napi_no_external_buffers_allowed` to indicate that external +buffers are not supported. One such runtime is Electron as +described in this issue +[electron/issues/35801](https://github.com/electron/electron/issues/35801). + +In order to maintain broadest compatibility with all runtimes, +you may define `NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED` in your addon before +includes for the node-api and node-addon-api headers. Doing so will hide the +functions that create external buffers. This will ensure a compilation error +occurs if you accidentally use one of these methods. + +In node-addon-api, the `Napi::Buffer::NewOrCopy` provides a convenient way to +create an external buffer, or allocate a new buffer and copy the data when the +external buffer is not supported. diff --git a/napi-inl.h b/napi-inl.h index 3eab2f32e..a800dd171 100644 --- a/napi-inl.h +++ b/napi-inl.h @@ -22,9 +22,13 @@ namespace Napi { namespace NAPI_CPP_CUSTOM_NAMESPACE { #endif -// Helpers to handle functions exposed from C++. +// Helpers to handle functions exposed from C++ and internal constants. namespace details { +// New napi_status constants not yet available in all supported versions of +// Node.js releases. Only necessary when they are used in napi.h and napi-inl.h. +constexpr int napi_no_external_buffers_allowed = 22; + // Attach a data item to an object and delete it when the object gets // garbage-collected. // TODO: Replace this code with `napi_add_finalizer()` whenever it becomes @@ -1756,6 +1760,7 @@ inline ArrayBuffer ArrayBuffer::New(napi_env env, size_t byteLength) { return ArrayBuffer(env, value); } +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED inline ArrayBuffer ArrayBuffer::New(napi_env env, void* externalData, size_t byteLength) { @@ -1815,6 +1820,7 @@ inline ArrayBuffer ArrayBuffer::New(napi_env env, return ArrayBuffer(env, value); } +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED inline ArrayBuffer::ArrayBuffer() : Object() {} @@ -2434,6 +2440,7 @@ inline Buffer Buffer::New(napi_env env, size_t length) { return Buffer(env, value, length, static_cast(data)); } +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED template inline Buffer Buffer::New(napi_env env, T* data, size_t length) { napi_value value; @@ -2491,6 +2498,94 @@ inline Buffer Buffer::New(napi_env env, } return Buffer(env, value, length, data); } +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + +template +inline Buffer Buffer::NewOrCopy(napi_env env, T* data, size_t length) { +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + napi_value value; + napi_status status = napi_create_external_buffer( + env, length * sizeof(T), data, nullptr, nullptr, &value); + if (status == details::napi_no_external_buffers_allowed) { +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + // If we can't create an external buffer, we'll just copy the data. + return Buffer::Copy(env, data, length); +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + } + NAPI_THROW_IF_FAILED(env, status, Buffer()); + return Buffer(env, value, length, data); +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED +} + +template +template +inline Buffer Buffer::NewOrCopy(napi_env env, + T* data, + size_t length, + Finalizer finalizeCallback) { + details::FinalizeData* finalizeData = + new details::FinalizeData( + {std::move(finalizeCallback), nullptr}); +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + napi_value value; + napi_status status = + napi_create_external_buffer(env, + length * sizeof(T), + data, + details::FinalizeData::Wrapper, + finalizeData, + &value); + if (status == details::napi_no_external_buffers_allowed) { +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + // If we can't create an external buffer, we'll just copy the data. + Buffer ret = Buffer::Copy(env, data, length); + details::FinalizeData::Wrapper(env, data, finalizeData); + return ret; +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + } + if (status != napi_ok) { + delete finalizeData; + NAPI_THROW_IF_FAILED(env, status, Buffer()); + } + return Buffer(env, value, length, data); +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED +} + +template +template +inline Buffer Buffer::NewOrCopy(napi_env env, + T* data, + size_t length, + Finalizer finalizeCallback, + Hint* finalizeHint) { + details::FinalizeData* finalizeData = + new details::FinalizeData( + {std::move(finalizeCallback), finalizeHint}); +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + napi_value value; + napi_status status = napi_create_external_buffer( + env, + length * sizeof(T), + data, + details::FinalizeData::WrapperWithHint, + finalizeData, + &value); + if (status == details::napi_no_external_buffers_allowed) { +#endif + // If we can't create an external buffer, we'll just copy the data. + Buffer ret = Buffer::Copy(env, data, length); + details::FinalizeData::WrapperWithHint( + env, data, finalizeData); + return ret; +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + } + if (status != napi_ok) { + delete finalizeData; + NAPI_THROW_IF_FAILED(env, status, Buffer()); + } + return Buffer(env, value, length, data); +#endif +} template inline Buffer Buffer::Copy(napi_env env, const T* data, size_t length) { diff --git a/napi.h b/napi.h index b5f2a508a..82ba6aad0 100644 --- a/napi.h +++ b/napi.h @@ -1077,6 +1077,7 @@ class ArrayBuffer : public Object { size_t byteLength ///< Length of the buffer to be allocated, in bytes ); +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED /// Creates a new ArrayBuffer instance, using an external buffer with /// specified byte length. static ArrayBuffer New( @@ -1120,6 +1121,7 @@ class ArrayBuffer : public Object { Hint* finalizeHint ///< Hint (second parameter) to be passed to the ///< finalize callback ); +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED ArrayBuffer(); ///< Creates a new _empty_ ArrayBuffer instance. ArrayBuffer(napi_env env, @@ -1432,6 +1434,7 @@ template class Buffer : public Uint8Array { public: static Buffer New(napi_env env, size_t length); +#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED static Buffer New(napi_env env, T* data, size_t length); // Finalizer must implement `void operator()(Env env, T* data)`. @@ -1447,6 +1450,22 @@ class Buffer : public Uint8Array { size_t length, Finalizer finalizeCallback, Hint* finalizeHint); +#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED + + static Buffer NewOrCopy(napi_env env, T* data, size_t length); + // Finalizer must implement `void operator()(Env env, T* data)`. + template + static Buffer NewOrCopy(napi_env env, + T* data, + size_t length, + Finalizer finalizeCallback); + // Finalizer must implement `void operator()(Env env, T* data, Hint* hint)`. + template + static Buffer NewOrCopy(napi_env env, + T* data, + size_t length, + Finalizer finalizeCallback, + Hint* finalizeHint); static Buffer Copy(napi_env env, const T* data, size_t length); diff --git a/test/binding.cc b/test/binding.cc index 4d6985681..f0aaa3c2a 100644 --- a/test/binding.cc +++ b/test/binding.cc @@ -22,6 +22,7 @@ Object InitBasicTypesValue(Env env); Object InitBigInt(Env env); #endif Object InitBuffer(Env env); +Object InitBufferNoExternal(Env env); #if (NAPI_VERSION > 2) Object InitCallbackScope(Env env); #endif @@ -107,6 +108,7 @@ Object Init(Env env, Object exports) { exports.Set("date", InitDate(env)); #endif exports.Set("buffer", InitBuffer(env)); + exports.Set("bufferNoExternal", InitBufferNoExternal(env)); #if (NAPI_VERSION > 2) exports.Set("callbackscope", InitCallbackScope(env)); #endif diff --git a/test/binding.gyp b/test/binding.gyp index 1d21de979..bc426847e 100644 --- a/test/binding.gyp +++ b/test/binding.gyp @@ -20,6 +20,7 @@ 'callbackInfo.cc', 'date.cc', 'binding.cc', + 'buffer_no_external.cc', 'buffer.cc', 'callbackscope.cc', 'dataview/dataview.cc', diff --git a/test/buffer.cc b/test/buffer.cc index 20ff20f46..c10300dc7 100644 --- a/test/buffer.cc +++ b/test/buffer.cc @@ -1,30 +1,16 @@ +#include "buffer.h" #include "napi.h" using namespace Napi; -namespace { - -const size_t testLength = 4; +namespace test_buffer { uint16_t testData[testLength]; int finalizeCount = 0; +} // namespace test_buffer -template -void InitData(T* data, size_t length) { - for (size_t i = 0; i < length; i++) { - data[i] = static_cast(i); - } -} - -template -bool VerifyData(T* data, size_t length) { - for (size_t i = 0; i < length; i++) { - if (data[i] != static_cast(i)) { - return false; - } - } - return true; -} +using namespace test_buffer; +namespace { Value CreateBuffer(const CallbackInfo& info) { Buffer buffer = Buffer::New(info.Env(), testLength); @@ -146,6 +132,8 @@ Value CreateBufferCopy(const CallbackInfo& info) { return buffer; } +#include "buffer_new_or_copy-inl.h" + void CheckBuffer(const CallbackInfo& info) { if (!info[0].IsBuffer()) { Error::New(info.Env(), "A buffer was expected.") @@ -183,6 +171,12 @@ Object InitBuffer(Env env) { Function::New(env, CreateExternalBufferWithFinalize); exports["createExternalBufferWithFinalizeHint"] = Function::New(env, CreateExternalBufferWithFinalizeHint); + exports["createOrCopyExternalBuffer"] = + Function::New(env, CreateOrCopyExternalBuffer); + exports["createOrCopyExternalBufferWithFinalize"] = + Function::New(env, CreateOrCopyExternalBufferWithFinalize); + exports["createOrCopyExternalBufferWithFinalizeHint"] = + Function::New(env, CreateOrCopyExternalBufferWithFinalizeHint); exports["createBufferCopy"] = Function::New(env, CreateBufferCopy); exports["checkBuffer"] = Function::New(env, CheckBuffer); exports["getFinalizeCount"] = Function::New(env, GetFinalizeCount); diff --git a/test/buffer.h b/test/buffer.h new file mode 100644 index 000000000..ed2a71771 --- /dev/null +++ b/test/buffer.h @@ -0,0 +1,26 @@ +#include +#include + +namespace test_buffer { + +const size_t testLength = 4; +extern uint16_t testData[testLength]; +extern int finalizeCount; + +template +void InitData(T* data, size_t length) { + for (size_t i = 0; i < length; i++) { + data[i] = static_cast(i); + } +} + +template +bool VerifyData(T* data, size_t length) { + for (size_t i = 0; i < length; i++) { + if (data[i] != static_cast(i)) { + return false; + } + } + return true; +} +} // namespace test_buffer diff --git a/test/buffer.js b/test/buffer.js index 8b915bea9..8b190b79f 100644 --- a/test/buffer.js +++ b/test/buffer.js @@ -62,6 +62,88 @@ function test (binding) { () => { global.gc(); }, + () => { + assert.strictEqual(1, binding.buffer.getFinalizeCount()); + }, + + 'Create or Copy External Buffer', + () => { + const test = binding.buffer.createOrCopyExternalBuffer(); + binding.buffer.checkBuffer(test); + assert.ok(test instanceof Buffer); + assert.strictEqual(0, binding.buffer.getFinalizeCount()); + }, + () => { + global.gc(); + assert.strictEqual(0, binding.buffer.getFinalizeCount()); + }, + + 'Create or Copy External Buffer with finalizer', + () => { + const test = binding.buffer.createOrCopyExternalBufferWithFinalize(); + binding.buffer.checkBuffer(test); + assert.ok(test instanceof Buffer); + assert.strictEqual(0, binding.buffer.getFinalizeCount()); + }, + () => { + global.gc(); + }, + () => { + assert.strictEqual(1, binding.buffer.getFinalizeCount()); + }, + + 'Create or Copy External Buffer with finalizer hint', + () => { + const test = binding.buffer.createOrCopyExternalBufferWithFinalizeHint(); + binding.buffer.checkBuffer(test); + assert.ok(test instanceof Buffer); + assert.strictEqual(0, binding.buffer.getFinalizeCount()); + }, + () => { + global.gc(); + }, + () => { + assert.strictEqual(1, binding.buffer.getFinalizeCount()); + }, + + 'Create or Copy External Buffer when NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED defined', + () => { + const test = binding.bufferNoExternal.createOrCopyExternalBuffer(); + binding.buffer.checkBuffer(test); + assert.ok(test instanceof Buffer); + assert.strictEqual(0, binding.buffer.getFinalizeCount()); + }, + () => { + global.gc(); + assert.strictEqual(0, binding.buffer.getFinalizeCount()); + }, + + 'Create or Copy External Buffer with finalizer when NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED defined', + () => { + const test = binding.bufferNoExternal.createOrCopyExternalBufferWithFinalize(); + binding.buffer.checkBuffer(test); + assert.ok(test instanceof Buffer); + // finalizer should have been called when the buffer was created. + assert.strictEqual(1, binding.buffer.getFinalizeCount()); + }, + () => { + global.gc(); + }, + () => { + assert.strictEqual(1, binding.buffer.getFinalizeCount()); + }, + + 'Create or Copy External Buffer with finalizer hint when NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED defined', + () => { + const test = binding.bufferNoExternal.createOrCopyExternalBufferWithFinalizeHint(); + binding.buffer.checkBuffer(test); + assert.ok(test instanceof Buffer); + // finalizer should have been called when the buffer was created. + assert.strictEqual(1, binding.buffer.getFinalizeCount()); + }, + () => { + global.gc(); + }, () => { assert.strictEqual(1, binding.buffer.getFinalizeCount()); } diff --git a/test/buffer_new_or_copy-inl.h b/test/buffer_new_or_copy-inl.h new file mode 100644 index 000000000..4d68fbc91 --- /dev/null +++ b/test/buffer_new_or_copy-inl.h @@ -0,0 +1,68 @@ +// Same tests on when NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED is defined or not +// defined. + +Value CreateOrCopyExternalBuffer(const CallbackInfo& info) { + finalizeCount = 0; + + InitData(testData, testLength); + Buffer buffer = + Buffer::NewOrCopy(info.Env(), testData, testLength); + + if (buffer.Length() != testLength) { + Error::New(info.Env(), "Incorrect buffer length.") + .ThrowAsJavaScriptException(); + return Value(); + } + + VerifyData(buffer.Data(), testLength); + return buffer; +} + +Value CreateOrCopyExternalBufferWithFinalize(const CallbackInfo& info) { + finalizeCount = 0; + + uint16_t* data = new uint16_t[testLength]; + InitData(data, testLength); + + Buffer buffer = Buffer::NewOrCopy( + info.Env(), data, testLength, [](Env /*env*/, uint16_t* finalizeData) { + delete[] finalizeData; + finalizeCount++; + }); + + if (buffer.Length() != testLength) { + Error::New(info.Env(), "Incorrect buffer length.") + .ThrowAsJavaScriptException(); + return Value(); + } + + VerifyData(buffer.Data(), testLength); + return buffer; +} + +Value CreateOrCopyExternalBufferWithFinalizeHint(const CallbackInfo& info) { + finalizeCount = 0; + + uint16_t* data = new uint16_t[testLength]; + InitData(data, testLength); + + char* hint = nullptr; + Buffer buffer = Buffer::NewOrCopy( + info.Env(), + data, + testLength, + [](Env /*env*/, uint16_t* finalizeData, char* /*finalizeHint*/) { + delete[] finalizeData; + finalizeCount++; + }, + hint); + + if (buffer.Length() != testLength) { + Error::New(info.Env(), "Incorrect buffer length.") + .ThrowAsJavaScriptException(); + return Value(); + } + + VerifyData(buffer.Data(), testLength); + return buffer; +} diff --git a/test/buffer_no_external.cc b/test/buffer_no_external.cc new file mode 100644 index 000000000..11920bf1c --- /dev/null +++ b/test/buffer_no_external.cc @@ -0,0 +1,24 @@ +#define NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED +// Should compile without errors +#include "buffer.h" +#include "napi.h" + +using namespace Napi; +using namespace test_buffer; + +namespace { +#include "buffer_new_or_copy-inl.h" +} + +Object InitBufferNoExternal(Env env) { + Object exports = Object::New(env); + + exports["createOrCopyExternalBuffer"] = + Function::New(env, CreateOrCopyExternalBuffer); + exports["createOrCopyExternalBufferWithFinalize"] = + Function::New(env, CreateOrCopyExternalBufferWithFinalize); + exports["createOrCopyExternalBufferWithFinalizeHint"] = + Function::New(env, CreateOrCopyExternalBufferWithFinalizeHint); + + return exports; +}