Skip to content
Permalink
Browse files

src: simplify AliasedBuffer lifetime management

Rely on the V8 garbage collector to take care of managing
the lifetime of the underlying memory of an `AliasedBuffer`.

PR-URL: #26196
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Minwoo Jung <minwoo@nodesource.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information...
addaleax committed Feb 18, 2019
1 parent 6fb7baf commit d1011f6bbfadfa254925014896aa710f02403770
Showing with 14 additions and 29 deletions.
  1. +14 −29 src/aliased_buffer.h
@@ -30,19 +30,17 @@ class AliasedBuffer {
AliasedBuffer(v8::Isolate* isolate, const size_t count)
: isolate_(isolate),
count_(count),
byte_offset_(0),
free_buffer_(true) {
byte_offset_(0) {
CHECK_GT(count, 0);
const v8::HandleScope handle_scope(isolate_);

const size_t size_in_bytes = sizeof(NativeT) * count;

// allocate native buffer
buffer_ = Calloc<NativeT>(count);
const size_t size_in_bytes =
MultiplyWithOverflowCheck(sizeof(NativeT), count);

// allocate v8 ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, buffer_, size_in_bytes);
isolate_, size_in_bytes);
buffer_ = static_cast<NativeT*>(ab->GetContents().Data());

// allocate v8 TypedArray
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
@@ -65,16 +63,16 @@ class AliasedBuffer {
v8::Uint8Array>& backing_buffer)
: isolate_(isolate),
count_(count),
byte_offset_(byte_offset),
free_buffer_(false) {
byte_offset_(byte_offset) {
const v8::HandleScope handle_scope(isolate_);

v8::Local<v8::ArrayBuffer> ab = backing_buffer.GetArrayBuffer();

// validate that the byte_offset is aligned with sizeof(NativeT)
CHECK_EQ(byte_offset & (sizeof(NativeT) - 1), 0);
// validate this fits inside the backing buffer
CHECK_LE(sizeof(NativeT) * count, ab->ByteLength() - byte_offset);
CHECK_LE(MultiplyWithOverflowCheck(sizeof(NativeT), count),
ab->ByteLength() - byte_offset);

buffer_ = reinterpret_cast<NativeT*>(
const_cast<uint8_t*>(backing_buffer.GetNativeBuffer() + byte_offset));
@@ -87,25 +85,16 @@ class AliasedBuffer {
: isolate_(that.isolate_),
count_(that.count_),
byte_offset_(that.byte_offset_),
buffer_(that.buffer_),
free_buffer_(false) {
buffer_(that.buffer_) {
js_array_ = v8::Global<V8T>(that.isolate_, that.GetJSArray());
}

~AliasedBuffer() {
if (free_buffer_ && buffer_ != nullptr) {
free(buffer_);
}
js_array_.Reset();
}

AliasedBuffer& operator=(AliasedBuffer&& that) noexcept {
this->~AliasedBuffer();
isolate_ = that.isolate_;
count_ = that.count_;
byte_offset_ = that.byte_offset_;
buffer_ = that.buffer_;
free_buffer_ = that.free_buffer_;

js_array_.Reset(isolate_, that.js_array_.Get(isolate_));

@@ -231,29 +220,26 @@ class AliasedBuffer {
void reserve(size_t new_capacity) {
DCHECK_GE(new_capacity, count_);
DCHECK_EQ(byte_offset_, 0);
DCHECK(free_buffer_);
const v8::HandleScope handle_scope(isolate_);

const size_t old_size_in_bytes = sizeof(NativeT) * count_;
const size_t new_size_in_bytes = sizeof(NativeT) * new_capacity;

// allocate v8 new ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, new_size_in_bytes);

// allocate new native buffer
NativeT* new_buffer = Calloc<NativeT>(new_capacity);
NativeT* new_buffer = static_cast<NativeT*>(ab->GetContents().Data());
// copy old content
memcpy(new_buffer, buffer_, old_size_in_bytes);

// allocate v8 new ArrayBuffer
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
isolate_, new_buffer, new_size_in_bytes);

// allocate v8 TypedArray
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, new_capacity);

// move over old v8 TypedArray
js_array_ = std::move(v8::Global<V8T>(isolate_, js_array));

// Free old buffer and set new values
free(buffer_);
buffer_ = new_buffer;
count_ = new_capacity;
}
@@ -264,7 +250,6 @@ class AliasedBuffer {
size_t byte_offset_;
NativeT* buffer_;
v8::Global<V8T> js_array_;
bool free_buffer_;
};
} // namespace node

0 comments on commit d1011f6

Please sign in to comment.
You can’t perform that action at this time.