Skip to content

Commit e66a2ac

Browse files
addaleaxdanbev
addaleax
authored and
danbev
committed
src: migrate off ArrayBuffer::GetContents
V8 deprecates `GetContents()` in favour of `GetBackingStore()`. Update our code to reflect that. V8 also deprecates `Externalize()` and `IsExternal()`; we should be able to remove all usage of this once V8 8.0 is there. PR-URL: nodejs#30339 Refs: v8/v8@bfe3d6b Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: David Carlier <devnexen@gmail.com>
1 parent f8c069f commit e66a2ac

File tree

13 files changed

+67
-44
lines changed

13 files changed

+67
-44
lines changed

‎src/aliased_buffer.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ class AliasedBufferBase {
4242
// allocate v8 ArrayBuffer
4343
v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
4444
isolate_, size_in_bytes);
45-
buffer_ = static_cast<NativeT*>(ab->GetContents().Data());
45+
buffer_ = static_cast<NativeT*>(ab->GetBackingStore()->Data());
4646

4747
// allocate v8 TypedArray
4848
v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
@@ -228,7 +228,7 @@ class AliasedBufferBase {
228228
isolate_, new_size_in_bytes);
229229

230230
// allocate new native buffer
231-
NativeT* new_buffer = static_cast<NativeT*>(ab->GetContents().Data());
231+
NativeT* new_buffer = static_cast<NativeT*>(ab->GetBackingStore()->Data());
232232
// copy old content
233233
memcpy(new_buffer, buffer_, old_size_in_bytes);
234234

‎src/js_native_api_v8.cc‎

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2562,7 +2562,7 @@ napi_status napi_create_arraybuffer(napi_env env,
25622562
// Optionally return a pointer to the buffer's data, to avoid another call to
25632563
// retrieve it.
25642564
if (data != nullptr) {
2565-
*data = buffer->GetContents().Data();
2565+
*data = buffer->GetBackingStore()->Data();
25662566
}
25672567

25682568
*result = v8impl::JsValueFromV8LocalValue(buffer);
@@ -2608,15 +2608,15 @@ napi_status napi_get_arraybuffer_info(napi_env env,
26082608
v8::Local<v8::Value> value = v8impl::V8LocalValueFromJsValue(arraybuffer);
26092609
RETURN_STATUS_IF_FALSE(env, value->IsArrayBuffer(), napi_invalid_arg);
26102610

2611-
v8::ArrayBuffer::Contents contents =
2612-
value.As<v8::ArrayBuffer>()->GetContents();
2611+
std::shared_ptr<v8::BackingStore> backing_store =
2612+
value.As<v8::ArrayBuffer>()->GetBackingStore();
26132613

26142614
if (data != nullptr) {
2615-
*data = contents.Data();
2615+
*data = backing_store->Data();
26162616
}
26172617

26182618
if (byte_length != nullptr) {
2619-
*byte_length = contents.ByteLength();
2619+
*byte_length = backing_store->ByteLength();
26202620
}
26212621

26222622
return napi_clear_last_error(env);
@@ -2747,9 +2747,15 @@ napi_status napi_get_typedarray_info(napi_env env,
27472747
*length = array->Length();
27482748
}
27492749

2750-
v8::Local<v8::ArrayBuffer> buffer = array->Buffer();
2750+
v8::Local<v8::ArrayBuffer> buffer;
2751+
if (data != nullptr || arraybuffer != nullptr) {
2752+
// Calling Buffer() may have the side effect of allocating the buffer,
2753+
// so only do this when it’s needed.
2754+
buffer = array->Buffer();
2755+
}
2756+
27512757
if (data != nullptr) {
2752-
*data = static_cast<uint8_t*>(buffer->GetContents().Data()) +
2758+
*data = static_cast<uint8_t*>(buffer->GetBackingStore()->Data()) +
27532759
array->ByteOffset();
27542760
}
27552761

@@ -2821,9 +2827,15 @@ napi_status napi_get_dataview_info(napi_env env,
28212827
*byte_length = array->ByteLength();
28222828
}
28232829

2824-
v8::Local<v8::ArrayBuffer> buffer = array->Buffer();
2830+
v8::Local<v8::ArrayBuffer> buffer;
2831+
if (data != nullptr || arraybuffer != nullptr) {
2832+
// Calling Buffer() may have the side effect of allocating the buffer,
2833+
// so only do this when it’s needed.
2834+
buffer = array->Buffer();
2835+
}
2836+
28252837
if (data != nullptr) {
2826-
*data = static_cast<uint8_t*>(buffer->GetContents().Data()) +
2838+
*data = static_cast<uint8_t*>(buffer->GetBackingStore()->Data()) +
28272839
array->ByteOffset();
28282840
}
28292841

@@ -3015,6 +3027,7 @@ napi_status napi_detach_arraybuffer(napi_env env, napi_value arraybuffer) {
30153027
env, value->IsArrayBuffer(), napi_arraybuffer_expected);
30163028

30173029
v8::Local<v8::ArrayBuffer> it = value.As<v8::ArrayBuffer>();
3030+
// TODO(addaleax): Remove the first condition once we have V8 8.0.
30183031
RETURN_STATUS_IF_FALSE(
30193032
env, it->IsExternal(), napi_detachable_arraybuffer_expected);
30203033
RETURN_STATUS_IF_FALSE(

‎src/node_buffer.cc‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -192,16 +192,13 @@ bool HasInstance(Local<Object> obj) {
192192
char* Data(Local<Value> val) {
193193
CHECK(val->IsArrayBufferView());
194194
Local<ArrayBufferView> ui = val.As<ArrayBufferView>();
195-
ArrayBuffer::Contents ab_c = ui->Buffer()->GetContents();
196-
return static_cast<char*>(ab_c.Data()) + ui->ByteOffset();
195+
return static_cast<char*>(ui->Buffer()->GetBackingStore()->Data()) +
196+
ui->ByteOffset();
197197
}
198198

199199

200200
char* Data(Local<Object> obj) {
201-
CHECK(obj->IsArrayBufferView());
202-
Local<ArrayBufferView> ui = obj.As<ArrayBufferView>();
203-
ArrayBuffer::Contents ab_c = ui->Buffer()->GetContents();
204-
return static_cast<char*>(ab_c.Data()) + ui->ByteOffset();
201+
return Data(obj.As<Value>());
205202
}
206203

207204

@@ -1060,13 +1057,13 @@ static void EncodeInto(const FunctionCallbackInfo<Value>& args) {
10601057
Local<Uint8Array> dest = args[1].As<Uint8Array>();
10611058
Local<ArrayBuffer> buf = dest->Buffer();
10621059
char* write_result =
1063-
static_cast<char*>(buf->GetContents().Data()) + dest->ByteOffset();
1060+
static_cast<char*>(buf->GetBackingStore()->Data()) + dest->ByteOffset();
10641061
size_t dest_length = dest->ByteLength();
10651062

10661063
// results = [ read, written ]
10671064
Local<Uint32Array> result_arr = args[2].As<Uint32Array>();
10681065
uint32_t* results = reinterpret_cast<uint32_t*>(
1069-
static_cast<char*>(result_arr->Buffer()->GetContents().Data()) +
1066+
static_cast<char*>(result_arr->Buffer()->GetBackingStore()->Data()) +
10701067
result_arr->ByteOffset());
10711068

10721069
int nchars;

‎src/node_contextify.cc‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -697,8 +697,8 @@ void ContextifyScript::New(const FunctionCallbackInfo<Value>& args) {
697697

698698
ScriptCompiler::CachedData* cached_data = nullptr;
699699
if (!cached_data_buf.IsEmpty()) {
700-
ArrayBuffer::Contents contents = cached_data_buf->Buffer()->GetContents();
701-
uint8_t* data = static_cast<uint8_t*>(contents.Data());
700+
uint8_t* data = static_cast<uint8_t*>(
701+
cached_data_buf->Buffer()->GetBackingStore()->Data());
702702
cached_data = new ScriptCompiler::CachedData(
703703
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
704704
}
@@ -1044,8 +1044,8 @@ void ContextifyContext::CompileFunction(
10441044
// Read cache from cached data buffer
10451045
ScriptCompiler::CachedData* cached_data = nullptr;
10461046
if (!cached_data_buf.IsEmpty()) {
1047-
ArrayBuffer::Contents contents = cached_data_buf->Buffer()->GetContents();
1048-
uint8_t* data = static_cast<uint8_t*>(contents.Data());
1047+
uint8_t* data = static_cast<uint8_t*>(
1048+
cached_data_buf->Buffer()->GetBackingStore()->Data());
10491049
cached_data = new ScriptCompiler::CachedData(
10501050
data + cached_data_buf->ByteOffset(), cached_data_buf->ByteLength());
10511051
}

‎src/node_messaging.cc‎

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -298,12 +298,19 @@ Maybe<bool> Message::Serialize(Environment* env,
298298
// Currently, we support ArrayBuffers and MessagePorts.
299299
if (entry->IsArrayBuffer()) {
300300
Local<ArrayBuffer> ab = entry.As<ArrayBuffer>();
301-
// If we cannot render the ArrayBuffer unusable in this Isolate and
302-
// take ownership of its memory, copying the buffer will have to do.
303-
if (!ab->IsDetachable() || ab->IsExternal() ||
304-
!env->isolate_data()->uses_node_allocator()) {
301+
// If we cannot render the ArrayBuffer unusable in this Isolate,
302+
// copying the buffer will have to do.
303+
// Note that we can currently transfer ArrayBuffers even if they were
304+
// not allocated by Node’s ArrayBufferAllocator in the first place,
305+
// because we pass the underlying v8::BackingStore around rather than
306+
// raw data *and* an Isolate with a non-default ArrayBuffer allocator
307+
// is always going to outlive any Workers it creates, and so will its
308+
// allocator along with it.
309+
// TODO(addaleax): Eventually remove the IsExternal() condition,
310+
// see https://github.com/nodejs/node/pull/30339#issuecomment-552225353
311+
// for details.
312+
if (!ab->IsDetachable() || ab->IsExternal())
305313
continue;
306-
}
307314
if (std::find(array_buffers.begin(), array_buffers.end(), ab) !=
308315
array_buffers.end()) {
309316
ThrowDataCloneException(
@@ -363,7 +370,9 @@ Maybe<bool> Message::Serialize(Environment* env,
363370
for (Local<ArrayBuffer> ab : array_buffers) {
364371
// If serialization succeeded, we render it inaccessible in this Isolate.
365372
std::shared_ptr<BackingStore> backing_store = ab->GetBackingStore();
366-
ab->Externalize(backing_store);
373+
// TODO(addaleax): This can/should be dropped once we have V8 8.0.
374+
if (!ab->IsExternal())
375+
ab->Externalize(backing_store);
367376
ab->Detach();
368377

369378
array_buffers_.emplace_back(std::move(backing_store));

‎src/node_os.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ static void GetLoadAvg(const FunctionCallbackInfo<Value>& args) {
171171
Local<Float64Array> array = args[0].As<Float64Array>();
172172
CHECK_EQ(array->Length(), 3);
173173
Local<ArrayBuffer> ab = array->Buffer();
174-
double* loadavg = static_cast<double*>(ab->GetContents().Data());
174+
double* loadavg = static_cast<double*>(ab->GetBackingStore()->Data());
175175
uv_loadavg(loadavg);
176176
}
177177

‎src/node_process_methods.cc‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ static void CPUUsage(const FunctionCallbackInfo<Value>& args) {
109109
Local<Float64Array> array = args[0].As<Float64Array>();
110110
CHECK_EQ(array->Length(), 2);
111111
Local<ArrayBuffer> ab = array->Buffer();
112-
double* fields = static_cast<double*>(ab->GetContents().Data());
112+
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
113113

114114
// Set the Float64Array elements to be user / system values in microseconds.
115115
fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
@@ -148,7 +148,7 @@ static void Hrtime(const FunctionCallbackInfo<Value>& args) {
148148
uint64_t t = uv_hrtime();
149149

150150
Local<ArrayBuffer> ab = args[0].As<Uint32Array>()->Buffer();
151-
uint32_t* fields = static_cast<uint32_t*>(ab->GetContents().Data());
151+
uint32_t* fields = static_cast<uint32_t*>(ab->GetBackingStore()->Data());
152152

153153
fields[0] = (t / NANOS_PER_SEC) >> 32;
154154
fields[1] = (t / NANOS_PER_SEC) & 0xffffffff;
@@ -157,7 +157,7 @@ static void Hrtime(const FunctionCallbackInfo<Value>& args) {
157157

158158
static void HrtimeBigInt(const FunctionCallbackInfo<Value>& args) {
159159
Local<ArrayBuffer> ab = args[0].As<BigUint64Array>()->Buffer();
160-
uint64_t* fields = static_cast<uint64_t*>(ab->GetContents().Data());
160+
uint64_t* fields = static_cast<uint64_t*>(ab->GetBackingStore()->Data());
161161
fields[0] = uv_hrtime();
162162
}
163163

@@ -204,7 +204,7 @@ static void MemoryUsage(const FunctionCallbackInfo<Value>& args) {
204204
Local<Float64Array> array = args[0].As<Float64Array>();
205205
CHECK_EQ(array->Length(), 4);
206206
Local<ArrayBuffer> ab = array->Buffer();
207-
double* fields = static_cast<double*>(ab->GetContents().Data());
207+
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
208208

209209
fields[0] = rss;
210210
fields[1] = v8_heap_stats.total_heap_size();
@@ -301,7 +301,7 @@ static void ResourceUsage(const FunctionCallbackInfo<Value>& args) {
301301
Local<Float64Array> array = args[0].As<Float64Array>();
302302
CHECK_EQ(array->Length(), 16);
303303
Local<ArrayBuffer> ab = array->Buffer();
304-
double* fields = static_cast<double*>(ab->GetContents().Data());
304+
double* fields = static_cast<double*>(ab->GetBackingStore()->Data());
305305

306306
fields[0] = MICROS_PER_SEC * rusage.ru_utime.tv_sec + rusage.ru_utime.tv_usec;
307307
fields[1] = MICROS_PER_SEC * rusage.ru_stime.tv_sec + rusage.ru_stime.tv_usec;

‎src/node_worker.cc‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -622,7 +622,9 @@ void Worker::GetResourceLimits(const FunctionCallbackInfo<Value>& args) {
622622

623623
Local<Float64Array> Worker::GetResourceLimits(Isolate* isolate) const {
624624
Local<ArrayBuffer> ab = ArrayBuffer::New(isolate, sizeof(resource_limits_));
625-
memcpy(ab->GetContents().Data(), resource_limits_, sizeof(resource_limits_));
625+
memcpy(ab->GetBackingStore()->Data(),
626+
resource_limits_,
627+
sizeof(resource_limits_));
626628
return Float64Array::New(ab, 0, kTotalResourceLimitCount);
627629
}
628630

‎src/node_zlib.cc‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -590,7 +590,8 @@ class ZlibStream : public CompressionStream<ZlibContext> {
590590
CHECK(args[4]->IsUint32Array());
591591
Local<Uint32Array> array = args[4].As<Uint32Array>();
592592
Local<ArrayBuffer> ab = array->Buffer();
593-
uint32_t* write_result = static_cast<uint32_t*>(ab->GetContents().Data());
593+
uint32_t* write_result = static_cast<uint32_t*>(
594+
ab->GetBackingStore()->Data());
594595

595596
CHECK(args[5]->IsFunction());
596597
Local<Function> write_js_callback = args[5].As<Function>();

‎src/util-inl.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -514,7 +514,7 @@ void ArrayBufferViewContents<T, S>::Read(v8::Local<v8::ArrayBufferView> abv) {
514514
static_assert(sizeof(T) == 1, "Only supports one-byte data at the moment");
515515
length_ = abv->ByteLength();
516516
if (length_ > sizeof(stack_storage_) || abv->HasBuffer()) {
517-
data_ = static_cast<T*>(abv->Buffer()->GetContents().Data()) +
517+
data_ = static_cast<T*>(abv->Buffer()->GetBackingStore()->Data()) +
518518
abv->ByteOffset();
519519
} else {
520520
abv->CopyContents(stack_storage_, sizeof(stack_storage_));

0 commit comments

Comments
 (0)