Skip to content
This repository was archived by the owner on Mar 4, 2020. It is now read-only.

Commit dea1404

Browse files
addaleaxdeepak1556
authored andcommitted
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 f61bae3440e. It should render their downstream patch unnecessary. Refs: f61bae3 PR-URL: nodejs/node#26207 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent 4f63360 commit dea1404

15 files changed

+255
-291
lines changed

src/node_buffer.cc

+44-68
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,6 @@
5555
size_t length = end - start;
5656

5757
namespace node {
58-
59-
// if true, all Buffer and SlowBuffer instances will automatically zero-fill
60-
bool zero_fill_all_buffers = false;
61-
62-
namespace {
63-
64-
inline void* BufferMalloc(size_t length) {
65-
return zero_fill_all_buffers ? node::UncheckedCalloc(length) :
66-
node::UncheckedMalloc(length);
67-
}
68-
69-
} // namespace
70-
7158
namespace Buffer {
7259

7360
using v8::ArrayBuffer;
@@ -245,7 +232,7 @@ MaybeLocal<Object> New(Isolate* isolate,
245232
char* data = nullptr;
246233

247234
if (length > 0) {
248-
data = static_cast<char*>(BufferMalloc(length));
235+
data = UncheckedMalloc(length);
249236

250237
if (data == nullptr)
251238
return Local<Object>();
@@ -261,13 +248,7 @@ MaybeLocal<Object> New(Isolate* isolate,
261248
}
262249
}
263250

264-
Local<Object> buf;
265-
if (New(isolate, data, actual).ToLocal(&buf))
266-
return scope.Escape(buf);
267-
268-
// Object failed to be created. Clean up resources.
269-
free(data);
270-
return Local<Object>();
251+
return scope.EscapeMaybe(New(isolate, data, actual));
271252
}
272253

273254

@@ -290,28 +271,16 @@ MaybeLocal<Object> New(Environment* env, size_t length) {
290271
return Local<Object>();
291272
}
292273

293-
void* data;
274+
AllocatedBuffer ret(env);
294275
if (length > 0) {
295-
data = BufferMalloc(length);
296-
if (data == nullptr)
276+
ret = env->AllocateManaged(length, false);
277+
if (ret.data() == nullptr) {
278+
THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
297279
return Local<Object>();
298-
} else {
299-
data = nullptr;
300-
}
301-
302-
Local<ArrayBuffer> ab =
303-
ArrayBuffer::New(env->isolate(),
304-
data,
305-
length,
306-
ArrayBufferCreationMode::kInternalized);
307-
MaybeLocal<Uint8Array> ui = Buffer::New(env, ab, 0, length);
308-
309-
if (ui.IsEmpty()) {
310-
// Object failed to be created. Clean up resources.
311-
free(data);
280+
}
312281
}
313282

314-
return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
283+
return scope.EscapeMaybe(ret.ToBuffer());
315284
}
316285

317286

@@ -334,30 +303,18 @@ MaybeLocal<Object> Copy(Environment* env, const char* data, size_t length) {
334303
return Local<Object>();
335304
}
336305

337-
void* new_data;
306+
AllocatedBuffer ret(env);
338307
if (length > 0) {
339308
CHECK_NOT_NULL(data);
340-
new_data = node::UncheckedMalloc(length);
341-
if (new_data == nullptr)
309+
ret = env->AllocateManaged(length, false);
310+
if (ret.data() == nullptr) {
311+
THROW_ERR_MEMORY_ALLOCATION_FAILED(env);
342312
return Local<Object>();
343-
memcpy(new_data, data, length);
344-
} else {
345-
new_data = nullptr;
346-
}
347-
348-
Local<ArrayBuffer> ab =
349-
ArrayBuffer::New(env->isolate(),
350-
new_data,
351-
length,
352-
ArrayBufferCreationMode::kInternalized);
353-
MaybeLocal<Uint8Array> ui = Buffer::New(env, ab, 0, length);
354-
355-
if (ui.IsEmpty()) {
356-
// Object failed to be created. Clean up resources.
357-
free(new_data);
313+
}
314+
memcpy(ret.data(), data, length);
358315
}
359316

360-
return scope.Escape(ui.FromMaybe(Local<Uint8Array>()));
317+
return scope.EscapeMaybe(ret.ToBuffer());
361318
}
362319

363320

@@ -403,24 +360,44 @@ MaybeLocal<Object> New(Environment* env,
403360
return scope.Escape(ui.ToLocalChecked());
404361
}
405362

406-
363+
// Warning: This function needs `data` to be allocated with malloc() and not
364+
// necessarily isolate's ArrayBuffer::Allocator.
407365
MaybeLocal<Object> New(Isolate* isolate, char* data, size_t length) {
408366
EscapableHandleScope handle_scope(isolate);
409367
Environment* env = Environment::GetCurrent(isolate);
410368
CHECK_NOT_NULL(env); // TODO(addaleax): Handle nullptr here.
411369
Local<Object> obj;
412-
if (Buffer::New(env, data, length).ToLocal(&obj))
370+
if (Buffer::New(env, data, length, true).ToLocal(&obj))
413371
return handle_scope.Escape(obj);
414372
return Local<Object>();
415373
}
416374

417-
418-
MaybeLocal<Object> New(Environment* env, char* data, size_t length) {
375+
// Warning: If this call comes through the public node_buffer.h API,
376+
// the contract for this function is that `data` is allocated with malloc()
377+
// and not necessarily isolate's ArrayBuffer::Allocator.
378+
MaybeLocal<Object> New(Environment* env,
379+
char* data,
380+
size_t length,
381+
bool uses_malloc) {
419382
if (length > 0) {
420383
CHECK_NOT_NULL(data);
421384
CHECK(length <= kMaxLength);
422385
}
423386

387+
if (uses_malloc) {
388+
if (!env->isolate_data()->uses_node_allocator()) {
389+
// We don't know for sure that the allocator is malloc()-based, so we need
390+
// to fall back to the FreeCallback variant.
391+
auto free_callback = [](char* data, void* hint) { free(data); };
392+
return New(env, data, length, free_callback, nullptr);
393+
} else {
394+
// This is malloc()-based, so we can acquire it into our own
395+
// ArrayBufferAllocator.
396+
CHECK_NOT_NULL(env->isolate_data()->node_allocator());
397+
env->isolate_data()->node_allocator()->RegisterPointer(data, length);
398+
}
399+
}
400+
424401
Local<ArrayBuffer> ab =
425402
ArrayBuffer::New(env->isolate(),
426403
data,
@@ -1020,15 +997,13 @@ static void EncodeUtf8String(const FunctionCallbackInfo<Value>& args) {
1020997

1021998
Local<String> str = args[0].As<String>();
1022999
size_t length = str->Utf8Length(isolate);
1023-
char* data = node::UncheckedMalloc(length);
1000+
AllocatedBuffer buf = env->AllocateManaged(length);
10241001
str->WriteUtf8(isolate,
1025-
data,
1002+
buf.data(),
10261003
-1, // We are certain that `data` is sufficiently large
10271004
nullptr,
10281005
String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8);
1029-
auto array_buf = ArrayBuffer::New(
1030-
isolate, data, length, ArrayBufferCreationMode::kInternalized);
1031-
auto array = Uint8Array::New(array_buf, 0, length);
1006+
auto array = Uint8Array::New(buf.ToArrayBuffer(), 0, length);
10321007
args.GetReturnValue().Set(array);
10331008
}
10341009

@@ -1055,7 +1030,8 @@ void SetupBufferJS(const FunctionCallbackInfo<Value>& args) {
10551030
env->SetMethod(proto, "ucs2Write", StringWrite<UCS2>);
10561031
env->SetMethod(proto, "utf8Write", StringWrite<UTF8>);
10571032

1058-
if (auto zero_fill_field = env->isolate_data()->zero_fill_field()) {
1033+
if (ArrayBufferAllocator* allocator = env->isolate_data()->node_allocator()) {
1034+
uint32_t* zero_fill_field = allocator->zero_fill_field();
10591035
CHECK(args[1]->IsObject());
10601036
auto binding_object = args[1].As<Object>();
10611037
auto array_buffer = ArrayBuffer::New(env->isolate(),

0 commit comments

Comments
 (0)