From 427fce711f864f206a52ee8883442d443a8c4d34 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 10 Apr 2019 10:53:54 +0200 Subject: [PATCH] =?UTF-8?q?src:=20fix=20check=20for=20accepting=20Buffers?= =?UTF-8?q?=20into=20Node=E2=80=99s=20allocator?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This condition was incorrect. We currently take the fallback path in default Node builds, which always works, but may come with some overhead, whereas the intention was that we use the fast path in this condition. This is causing issues for embedders, because we would erroneously try to take the fast path when they don’t provide a Node.js-style `ArrayBufferAlloactor`, and crash as a consequence of that. This also requires us to relax the check in the debugging ArrayBuffer allocator a bit, because since d117e41e50667d7a36, 0-sized ArrayBuffers may actually point to allocations of size 1. Previously, that wasn’t caught because the fallback path circumvented our ArrayBufferAllocator. Refs: https://github.com/nodejs/node/commit/84e02b178ad14fae0df2a514e8a39bfa50ffdc2d#r33116006 PR-URL: https://github.com/nodejs/node/pull/27174 Reviewed-By: Joyee Cheung Reviewed-By: Richard Lau Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Gus Caplan --- src/api/environment.cc | 6 +++++- src/node_buffer.cc | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/api/environment.cc b/src/api/environment.cc index a87f591e4edf57..70228b77d8860b 100644 --- a/src/api/environment.cc +++ b/src/api/environment.cc @@ -139,7 +139,11 @@ void DebuggingArrayBufferAllocator::UnregisterPointerInternal(void* data, if (data == nullptr) return; auto it = allocations_.find(data); CHECK_NE(it, allocations_.end()); - CHECK_EQ(it->second, size); + if (size > 0) { + // We allow allocations with size 1 for 0-length buffers to avoid having + // to deal with nullptr values. + CHECK_EQ(it->second, size); + } allocations_.erase(it); } diff --git a/src/node_buffer.cc b/src/node_buffer.cc index 19d2de96b1c057..58175a8fd5f615 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -420,7 +420,7 @@ MaybeLocal New(Environment* env, } if (uses_malloc) { - if (env->isolate_data()->uses_node_allocator()) { + if (!env->isolate_data()->uses_node_allocator()) { // We don't know for sure that the allocator is malloc()-based, so we need // to fall back to the FreeCallback variant. auto free_callback = [](char* data, void* hint) { free(data); };