Skip to content

Commit 7e26ca6

Browse files
addaleaxrvagg
authored andcommitted
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>
1 parent 4bf58ac commit 7e26ca6

File tree

1 file changed

+14
-29
lines changed

1 file changed

+14
-29
lines changed

src/aliased_buffer.h

+14-29
Original file line numberDiff line numberDiff line change
@@ -30,19 +30,17 @@ class AliasedBuffer {
3030
AliasedBuffer(v8::Isolate* isolate, const size_t count)
3131
: isolate_(isolate),
3232
count_(count),
33-
byte_offset_(0),
34-
free_buffer_(true) {
33+
byte_offset_(0) {
3534
CHECK_GT(count, 0);
3635
const v8::HandleScope handle_scope(isolate_);
3736

38-
const size_t size_in_bytes = sizeof(NativeT) * count;
39-
40-
// allocate native buffer
41-
buffer_ = Calloc<NativeT>(count);
37+
const size_t size_in_bytes =
38+
MultiplyWithOverflowCheck(sizeof(NativeT), count);
4239

4340
// allocate v8 ArrayBuffer
4441
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
45-
isolate_, buffer_, size_in_bytes);
42+
isolate_, size_in_bytes);
43+
buffer_ = static_cast<NativeT*>(ab->GetContents().Data());
4644

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

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

7471
// validate that the byte_offset is aligned with sizeof(NativeT)
7572
CHECK_EQ(byte_offset & (sizeof(NativeT) - 1), 0);
7673
// validate this fits inside the backing buffer
77-
CHECK_LE(sizeof(NativeT) * count, ab->ByteLength() - byte_offset);
74+
CHECK_LE(MultiplyWithOverflowCheck(sizeof(NativeT), count),
75+
ab->ByteLength() - byte_offset);
7876

7977
buffer_ = reinterpret_cast<NativeT*>(
8078
const_cast<uint8_t*>(backing_buffer.GetNativeBuffer() + byte_offset));
@@ -87,25 +85,16 @@ class AliasedBuffer {
8785
: isolate_(that.isolate_),
8886
count_(that.count_),
8987
byte_offset_(that.byte_offset_),
90-
buffer_(that.buffer_),
91-
free_buffer_(false) {
88+
buffer_(that.buffer_) {
9289
js_array_ = v8::Global<V8T>(that.isolate_, that.GetJSArray());
9390
}
9491

95-
~AliasedBuffer() {
96-
if (free_buffer_ && buffer_ != nullptr) {
97-
free(buffer_);
98-
}
99-
js_array_.Reset();
100-
}
101-
10292
AliasedBuffer& operator=(AliasedBuffer&& that) noexcept {
10393
this->~AliasedBuffer();
10494
isolate_ = that.isolate_;
10595
count_ = that.count_;
10696
byte_offset_ = that.byte_offset_;
10797
buffer_ = that.buffer_;
108-
free_buffer_ = that.free_buffer_;
10998

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

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

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

228+
// allocate v8 new ArrayBuffer
229+
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
230+
isolate_, new_size_in_bytes);
231+
240232
// allocate new native buffer
241-
NativeT* new_buffer = Calloc<NativeT>(new_capacity);
233+
NativeT* new_buffer = static_cast<NativeT*>(ab->GetContents().Data());
242234
// copy old content
243235
memcpy(new_buffer, buffer_, old_size_in_bytes);
244236

245-
// allocate v8 new ArrayBuffer
246-
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
247-
isolate_, new_buffer, new_size_in_bytes);
248-
249237
// allocate v8 TypedArray
250238
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, new_capacity);
251239

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

255-
// Free old buffer and set new values
256-
free(buffer_);
257243
buffer_ = new_buffer;
258244
count_ = new_capacity;
259245
}
@@ -264,7 +250,6 @@ class AliasedBuffer {
264250
size_t byte_offset_;
265251
NativeT* buffer_;
266252
v8::Global<V8T> js_array_;
267-
bool free_buffer_;
268253
};
269254
} // namespace node
270255

0 commit comments

Comments
 (0)