Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

src: handle no support for external buffers #1273

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 10 additions & 0 deletions doc/array_buffer.md
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
97 changes: 97 additions & 0 deletions doc/buffer.md
Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
Expand All @@ -98,6 +107,93 @@ static Napi::Buffer<T> 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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
[external buffer][] is not supported, allocates a new `Napi::Buffer` object and
[external buffer][] is not supported by the underlying runtime, 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<T> 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 <typename Finalizer>
static Napi::Buffer<T> 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 <typename Finalizer, typename Hint>
static Napi::Buffer<T> 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.
Expand Down Expand Up @@ -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
18 changes: 18 additions & 0 deletions 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.
97 changes: 96 additions & 1 deletion napi-inl.h
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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() {}

Expand Down Expand Up @@ -2434,6 +2440,7 @@ inline Buffer<T> Buffer<T>::New(napi_env env, size_t length) {
return Buffer(env, value, length, static_cast<T*>(data));
}

#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
template <typename T>
inline Buffer<T> Buffer<T>::New(napi_env env, T* data, size_t length) {
napi_value value;
Expand Down Expand Up @@ -2491,6 +2498,94 @@ inline Buffer<T> Buffer<T>::New(napi_env env,
}
return Buffer(env, value, length, data);
}
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED

template <typename T>
inline Buffer<T> Buffer<T>::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<T>::Copy(env, data, length);
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
}
NAPI_THROW_IF_FAILED(env, status, Buffer<T>());
return Buffer(env, value, length, data);
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
}

template <typename T>
template <typename Finalizer>
inline Buffer<T> Buffer<T>::NewOrCopy(napi_env env,
T* data,
size_t length,
Finalizer finalizeCallback) {
details::FinalizeData<T, Finalizer>* finalizeData =
new details::FinalizeData<T, Finalizer>(
{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<T, Finalizer>::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<T> ret = Buffer<T>::Copy(env, data, length);
details::FinalizeData<T, Finalizer>::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 <typename T>
template <typename Finalizer, typename Hint>
inline Buffer<T> Buffer<T>::NewOrCopy(napi_env env,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume its the templates that prevent us from having one function with all the parameters and the others calling that one with dummy/null parameters, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really. When the template parameter Hint is set, the Finalizer must accept a third parameter Hint too. However, we allow Finalizer to be like void operator()(Env env, T* data). So we have to distinguish if Hint is present.

T* data,
size_t length,
Finalizer finalizeCallback,
Hint* finalizeHint) {
details::FinalizeData<T, Finalizer, Hint>* finalizeData =
new details::FinalizeData<T, Finalizer, Hint>(
{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<T, Finalizer, Hint>::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<T> ret = Buffer<T>::Copy(env, data, length);
details::FinalizeData<T, Finalizer, Hint>::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 <typename T>
inline Buffer<T> Buffer<T>::Copy(napi_env env, const T* data, size_t length) {
Expand Down
19 changes: 19 additions & 0 deletions napi.h
Expand Up @@ -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(
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1432,6 +1434,7 @@ template <typename T>
class Buffer : public Uint8Array {
public:
static Buffer<T> New(napi_env env, size_t length);
#ifndef NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED
static Buffer<T> New(napi_env env, T* data, size_t length);

// Finalizer must implement `void operator()(Env env, T* data)`.
Expand All @@ -1447,6 +1450,22 @@ class Buffer : public Uint8Array {
size_t length,
Finalizer finalizeCallback,
Hint* finalizeHint);
#endif // NODE_API_NO_EXTERNAL_BUFFERS_ALLOWED

static Buffer<T> NewOrCopy(napi_env env, T* data, size_t length);
// Finalizer must implement `void operator()(Env env, T* data)`.
template <typename Finalizer>
static Buffer<T> NewOrCopy(napi_env env,
T* data,
size_t length,
Finalizer finalizeCallback);
// Finalizer must implement `void operator()(Env env, T* data, Hint* hint)`.
template <typename Finalizer, typename Hint>
static Buffer<T> NewOrCopy(napi_env env,
T* data,
size_t length,
Finalizer finalizeCallback,
Hint* finalizeHint);

static Buffer<T> Copy(napi_env env, const T* data, size_t length);

Expand Down
2 changes: 2 additions & 0 deletions test/binding.cc
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/binding.gyp
Expand Up @@ -20,6 +20,7 @@
'callbackInfo.cc',
'date.cc',
'binding.cc',
'buffer_no_external.cc',
'buffer.cc',
'callbackscope.cc',
'dataview/dataview.cc',
Expand Down