Skip to content

Commit 3c11b4e

Browse files
committed
src: allocate Buffer memory using ArrayBuffer allocator
Always use the right allocator for memory that is turned into an `ArrayBuffer` at a later point. This enables embedders to use their own `ArrayBuffer::Allocator`s, and is inspired by Electron’s electron/node@f61bae3440e. It should render their downstream patch unnecessary. Refs: electron/node@f61bae3 PR-URL: #26207 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]> Backport-PR-URL: #26302 Reviewed-By: Michaël Zasso <[email protected]>
1 parent 2826076 commit 3c11b4e

16 files changed

+258
-327
lines changed

src/node_buffer.cc

+42-66
Original file line numberDiff line numberDiff line change
@@ -54,17 +54,6 @@
5454
size_t length = end - start;
5555

5656
namespace node {
57-
58-
namespace {
59-
60-
inline void* BufferMalloc(size_t length) {
61-
return per_process::cli_options->zero_fill_all_buffers ?
62-
node::UncheckedCalloc(length) :
63-
node::UncheckedMalloc(length);
64-
}
65-
66-
} // namespace
67-
6857
namespace Buffer {
6958

7059
using v8::ArrayBuffer;
@@ -260,7 +249,7 @@ MaybeLocal<Object> New(Isolate* isolate,
260249
char* data = nullptr;
261250

262251
if (length > 0) {
263-
data = static_cast<char*>(BufferMalloc(length));
252+
data = UncheckedMalloc(length);
264253

265254
if (data == nullptr)
266255
return Local<Object>();
@@ -276,13 +265,7 @@ MaybeLocal<Object> New(Isolate* isolate,
276265
}
277266
}
278267

279-
Local<Object> buf;
280-
if (New(isolate, data, actual).ToLocal(&buf))
281-
return scope.Escape(buf);
282-
283-
// Object failed to be created. Clean up resources.
284-
free(data);
285-
return Local<Object>();
268+
return scope.EscapeMaybe(New(isolate, data, actual));
286269
}
287270

288271

@@ -308,28 +291,15 @@ MaybeLocal<Object> New(Environment* env, size_t length) {
308291
return Local<Object>();
309292
}
310293

311-
void* data;
294+
AllocatedBuffer ret(env);
312295
if (length > 0) {
313-
data = BufferMalloc(length);
314-
if (data == nullptr)
296+
ret = env->AllocateManaged(length, false);
297+
if (ret.data() == nullptr) {
315298
return Local<Object>();
316-
} else {
317-
data = nullptr;
318-
}
319-
320-
Local<ArrayBuffer> ab =
321-
ArrayBuffer::New(env->isolate(),
322-
data,
323-
length,
324-
ArrayBufferCreationMode::kInternalized);
325-
MaybeLocal<Uint8Array> ui = Buffer::New(env, ab, 0, length);
326-
327-
if (ui.IsEmpty()) {
328-
// Object failed to be created. Clean up resources.
329-
free(data);
299+
}
330300
}
331301

332-
return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
302+
return scope.EscapeMaybe(ret.ToBuffer());
333303
}
334304

335305

@@ -355,30 +325,17 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
355325
return Local<Object>();
356326
}
357327

358-
void* new_data;
328+
AllocatedBuffer ret(env);
359329
if (length > 0) {
360330
CHECK_NOT_NULL(data);
361-
new_data = node::UncheckedMalloc(length);
362-
if (new_data == nullptr)
331+
ret = env->AllocateManaged(length, false);
332+
if (ret.data() == nullptr) {
363333
return Local<Object>();
364-
memcpy(new_data, data, length);
365-
} else {
366-
new_data = nullptr;
367-
}
368-
369-
Local<ArrayBuffer> ab =
370-
ArrayBuffer::New(env->isolate(),
371-
new_data,
372-
length,
373-
ArrayBufferCreationMode::kInternalized);
374-
MaybeLocal<Uint8Array> ui = Buffer::New(env, ab, 0, length);
375-
376-
if (ui.IsEmpty()) {
377-
// Object failed to be created. Clean up resources.
378-
free(new_data);
334+
}
335+
memcpy(ret.data(), data, length);
379336
}
380337

381-
return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
338+
return scope.EscapeMaybe(ret.ToBuffer());
382339
}
383340

384341

@@ -423,7 +380,8 @@ MaybeLocal<Object> New(Environment* env,
423380
return scope.Escape(ui.ToLocalChecked());
424381
}
425382

426-
383+
// Warning: This function needs `data` to be allocated with malloc() and not
384+
// necessarily isolate's ArrayBuffer::Allocator.
427385
MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
428386
EscapableHandleScope handle_scope(isolate);
429387
Environment* env = Environment::GetCurrent(isolate);
@@ -433,18 +391,37 @@ MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
433391
return MaybeLocal<Object>();
434392
}
435393
Local<Object> obj;
436-
if (Buffer::New(env, data, length).ToLocal(&obj))
394+
if (Buffer::New(env, data, length, true).ToLocal(&obj))
437395
return handle_scope.Escape(obj);
438396
return Local<Object>();
439397
}
440398

441-
442-
MaybeLocal<Object> New(Environment* env, char* data, size_t length) {
399+
// Warning: If this call comes through the public node_buffer.h API,
400+
// the contract for this function is that `data` is allocated with malloc()
401+
// and not necessarily isolate's ArrayBuffer::Allocator.
402+
MaybeLocal<Object> New(Environment* env,
403+
char* data,
404+
size_t length,
405+
bool uses_malloc) {
443406
if (length > 0) {
444407
CHECK_NOT_NULL(data);
445408
CHECK(length <= kMaxLength);
446409
}
447410

411+
if (uses_malloc) {
412+
if (env->isolate_data()->uses_node_allocator()) {
413+
// We don't know for sure that the allocator is malloc()-based, so we need
414+
// to fall back to the FreeCallback variant.
415+
auto free_callback = [](char* data, void* hint) { free(data); };
416+
return New(env, data, length, free_callback, nullptr);
417+
} else {
418+
// This is malloc()-based, so we can acquire it into our own
419+
// ArrayBufferAllocator.
420+
CHECK_NOT_NULL(env->isolate_data()->node_allocator());
421+
env->isolate_data()->node_allocator()->RegisterPointer(data, length);
422+
}
423+
}
424+
448425
Local<ArrayBuffer> ab =
449426
ArrayBuffer::New(env->isolate(),
450427
data,
@@ -1051,15 +1028,13 @@ static void EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
10511028

10521029
Local<String> str = args[0].As<String>();
10531030
size_t length = str->Utf8Length(isolate);
1054-
char* data = node::UncheckedMalloc(length);
1031+
AllocatedBuffer buf = env->AllocateManaged(length);
10551032
str->WriteUtf8(isolate,
1056-
data,
1033+
buf.data(),
10571034
-1, // We are certain that `data` is sufficiently large
10581035
nullptr,
10591036
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8);
1060-
auto array_buf = ArrayBuffer::New(
1061-
isolate, data, length, ArrayBufferCreationMode::kInternalized);
1062-
auto array = Uint8Array::New(array_buf, 0, length);
1037+
auto array = Uint8Array::New(buf.ToArrayBuffer(), 0, length);
10631038
args.GetReturnValue().Set(array);
10641039
}
10651040

@@ -1121,7 +1096,8 @@ void Initialize(Local<Object> target,
11211096

11221097
// It can be a nullptr when running inside an isolate where we
11231098
// do not own the ArrayBuffer allocator.
1124-
if (uint32_t* zero_fill_field = env->isolate_data()->zero_fill_field()) {
1099+
if (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) {
1100+
uint32_t* zero_fill_field = allocator->zero_fill_field();
11251101
Local<ArrayBuffer> array_buffer = ArrayBuffer::New(
11261102
env->isolate(), zero_fill_field, sizeof(*zero_fill_field));
11271103
CHECK(target

0 commit comments

Comments
 (0)