Skip to content

src: fix broken fast callback signatures #59055

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 26 additions & 28 deletions src/histogram.cc
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "base_object-inl.h"
#include "histogram-inl.h"
#include "memory_tracker-inl.h"
#include "node_debug.h"
#include "node_errors.h"
#include "node_external_reference.h"
#include "util.h"
Expand All @@ -11,10 +12,8 @@ namespace node {
using v8::BigInt;
using v8::CFunction;
using v8::Context;
using v8::FastApiCallbackOptions;
using v8::FunctionCallbackInfo;
using v8::FunctionTemplate;
using v8::HandleScope;
using v8::Integer;
using v8::Isolate;
using v8::Local;
Expand Down Expand Up @@ -162,8 +161,8 @@ void HistogramBase::RecordDelta(const FunctionCallbackInfo<Value>& args) {
(*histogram)->RecordDelta();
}

void HistogramBase::FastRecordDelta(Local<Value> unused,
Local<Value> receiver) {
void HistogramBase::FastRecordDelta(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.recordDelta");
HistogramBase* histogram;
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
(*histogram)->RecordDelta();
Expand All @@ -183,15 +182,9 @@ void HistogramBase::Record(const FunctionCallbackInfo<Value>& args) {
(*histogram)->Record(value);
}

void HistogramBase::FastRecord(Local<Value> unused,
Local<Value> receiver,
const int64_t value,
FastApiCallbackOptions& options) {
if (value < 1) {
HandleScope scope(options.isolate);
THROW_ERR_OUT_OF_RANGE(options.isolate, "value is out of range");
return;
}
Comment on lines -190 to -194
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

value is validated in the JS layer, so a debug check should be fine here.

void HistogramBase::FastRecord(Local<Value> receiver, const int64_t value) {
DCHECK_GE(value, 1);
TRACK_V8_FAST_API_CALL("histogram.record");
HistogramBase* histogram;
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
(*histogram)->Record(value);
Expand Down Expand Up @@ -428,9 +421,8 @@ void IntervalHistogram::Start(const FunctionCallbackInfo<Value>& args) {
histogram->OnStart(args[0]->IsTrue() ? StartFlags::RESET : StartFlags::NONE);
}

void IntervalHistogram::FastStart(Local<Value> unused,
Local<Value> receiver,
bool reset) {
void IntervalHistogram::FastStart(Local<Value> receiver, bool reset) {
TRACK_V8_FAST_API_CALL("histogram.start");
IntervalHistogram* histogram;
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
histogram->OnStart(reset ? StartFlags::RESET : StartFlags::NONE);
Expand All @@ -442,7 +434,8 @@ void IntervalHistogram::Stop(const FunctionCallbackInfo<Value>& args) {
histogram->OnStop();
}

void IntervalHistogram::FastStop(Local<Value> unused, Local<Value> receiver) {
void IntervalHistogram::FastStop(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.stop");
IntervalHistogram* histogram;
ASSIGN_OR_RETURN_UNWRAP(&histogram, receiver);
histogram->OnStop();
Expand Down Expand Up @@ -558,46 +551,51 @@ void HistogramImpl::DoReset(const FunctionCallbackInfo<Value>& args) {
(*histogram)->Reset();
}

void HistogramImpl::FastReset(Local<Value> unused, Local<Value> receiver) {
void HistogramImpl::FastReset(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.reset");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
(*histogram)->Reset();
}

double HistogramImpl::FastGetCount(Local<Value> unused, Local<Value> receiver) {
double HistogramImpl::FastGetCount(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.count");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Count());
}

double HistogramImpl::FastGetMin(Local<Value> unused, Local<Value> receiver) {
double HistogramImpl::FastGetMin(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.min");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Min());
}

double HistogramImpl::FastGetMax(Local<Value> unused, Local<Value> receiver) {
double HistogramImpl::FastGetMax(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.max");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Max());
}

double HistogramImpl::FastGetMean(Local<Value> unused, Local<Value> receiver) {
double HistogramImpl::FastGetMean(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.mean");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return (*histogram)->Mean();
}

double HistogramImpl::FastGetExceeds(Local<Value> unused,
Local<Value> receiver) {
double HistogramImpl::FastGetExceeds(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.exceeds");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Exceeds());
}

double HistogramImpl::FastGetStddev(Local<Value> unused,
Local<Value> receiver) {
double HistogramImpl::FastGetStddev(Local<Value> receiver) {
TRACK_V8_FAST_API_CALL("histogram.stddev");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return (*histogram)->Stddev();
}

double HistogramImpl::FastGetPercentile(Local<Value> unused,
Local<Value> receiver,
double HistogramImpl::FastGetPercentile(Local<Value> receiver,
const double percentile) {
TRACK_V8_FAST_API_CALL("histogram.percentile");
HistogramImpl* histogram = HistogramImpl::FromJSObject(receiver);
return static_cast<double>((*histogram)->Percentile(percentile));
}
Expand Down
40 changes: 12 additions & 28 deletions src/histogram.h
Original file line number Diff line number Diff line change
Expand Up @@ -101,22 +101,14 @@ class HistogramImpl {
static void GetPercentilesBigInt(
const v8::FunctionCallbackInfo<v8::Value>& args);

static void FastReset(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetCount(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetMin(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetMax(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetMean(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetExceeds(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetStddev(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static double FastGetPercentile(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver,
static void FastReset(v8::Local<v8::Value> receiver);
static double FastGetCount(v8::Local<v8::Value> receiver);
static double FastGetMin(v8::Local<v8::Value> receiver);
static double FastGetMax(v8::Local<v8::Value> receiver);
static double FastGetMean(v8::Local<v8::Value> receiver);
static double FastGetExceeds(v8::Local<v8::Value> receiver);
static double FastGetStddev(v8::Local<v8::Value> receiver);
static double FastGetPercentile(v8::Local<v8::Value> receiver,
const double percentile);

static void AddMethods(v8::Isolate* isolate,
Expand Down Expand Up @@ -165,13 +157,8 @@ class HistogramBase final : public BaseObject, public HistogramImpl {
static void RecordDelta(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Add(const v8::FunctionCallbackInfo<v8::Value>& args);

static void FastRecord(
v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver,
const int64_t value,
v8::FastApiCallbackOptions& options); // NOLINT(runtime/references)
static void FastRecordDelta(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static void FastRecord(v8::Local<v8::Value> receiver, const int64_t value);
static void FastRecordDelta(v8::Local<v8::Value> receiver);

HistogramBase(
Environment* env,
Expand Down Expand Up @@ -243,11 +230,8 @@ class IntervalHistogram final : public HandleWrap, public HistogramImpl {
static void Start(const v8::FunctionCallbackInfo<v8::Value>& args);
static void Stop(const v8::FunctionCallbackInfo<v8::Value>& args);

static void FastStart(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver,
bool reset);
static void FastStop(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver);
static void FastStart(v8::Local<v8::Value> receiver, bool reset);
static void FastStop(v8::Local<v8::Value> receiver);

BaseObject::TransferMode GetTransferMode() const override {
return TransferMode::kCloneable;
Expand Down
25 changes: 13 additions & 12 deletions src/node_process.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

#include "node_debug.h"
#include "node_snapshotable.h"
#include "v8-fast-api-calls.h"
#include "v8.h"
Expand Down Expand Up @@ -72,23 +73,23 @@ class BindingData : public SnapshotableObject {
SET_SELF_SIZE(BindingData)

static BindingData* FromV8Value(v8::Local<v8::Value> receiver);
static void NumberImpl(BindingData* receiver);
static void HrtimeImpl(BindingData* receiver);

static void FastNumber(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver) {
NumberImpl(FromV8Value(receiver));
static void FastHrtime(v8::Local<v8::Value> receiver) {
TRACK_V8_FAST_API_CALL("process.hrtime");
HrtimeImpl(FromV8Value(receiver));
}

static void SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SlowHrtime(const v8::FunctionCallbackInfo<v8::Value>& args);

static void BigIntImpl(BindingData* receiver);
static void HrtimeBigIntImpl(BindingData* receiver);

static void FastBigInt(v8::Local<v8::Value> unused,
v8::Local<v8::Value> receiver) {
BigIntImpl(FromV8Value(receiver));
static void FastHrtimeBigInt(v8::Local<v8::Value> receiver) {
TRACK_V8_FAST_API_CALL("process.hrtimeBigInt");
HrtimeBigIntImpl(FromV8Value(receiver));
}

static void SlowBigInt(const v8::FunctionCallbackInfo<v8::Value>& args);
static void SlowHrtimeBigInt(const v8::FunctionCallbackInfo<v8::Value>& args);

static void LoadEnvFile(const v8::FunctionCallbackInfo<v8::Value>& args);

Expand All @@ -101,8 +102,8 @@ class BindingData : public SnapshotableObject {
// These need to be static so that we have their addresses available to
// register as external references in the snapshot at environment creation
// time.
static v8::CFunction fast_number_;
static v8::CFunction fast_bigint_;
static v8::CFunction fast_hrtime_;
static v8::CFunction fast_hrtime_bigint_;
};

} // namespace process
Expand Down
28 changes: 14 additions & 14 deletions src/node_process_methods.cc
Original file line number Diff line number Diff line change
Expand Up @@ -652,22 +652,22 @@ BindingData::BindingData(Realm* realm,
hrtime_buffer_.MakeWeak();
}

v8::CFunction BindingData::fast_number_(v8::CFunction::Make(FastNumber));
v8::CFunction BindingData::fast_bigint_(v8::CFunction::Make(FastBigInt));
CFunction BindingData::fast_hrtime_(CFunction::Make(FastHrtime));
CFunction BindingData::fast_hrtime_bigint_(CFunction::Make(FastHrtimeBigInt));

void BindingData::AddMethods(Isolate* isolate, Local<ObjectTemplate> target) {
SetFastMethodNoSideEffect(
isolate, target, "hrtime", SlowNumber, &fast_number_);
isolate, target, "hrtime", SlowHrtime, &fast_hrtime_);
SetFastMethodNoSideEffect(
isolate, target, "hrtimeBigInt", SlowBigInt, &fast_bigint_);
isolate, target, "hrtimeBigInt", SlowHrtimeBigInt, &fast_hrtime_bigint_);
}

void BindingData::RegisterExternalReferences(
ExternalReferenceRegistry* registry) {
registry->Register(SlowNumber);
registry->Register(SlowBigInt);
registry->Register(fast_number_);
registry->Register(fast_bigint_);
registry->Register(SlowHrtime);
registry->Register(SlowHrtimeBigInt);
registry->Register(fast_hrtime_);
registry->Register(fast_hrtime_bigint_);
}

BindingData* BindingData::FromV8Value(Local<Value> value) {
Expand All @@ -689,14 +689,14 @@ void BindingData::MemoryInfo(MemoryTracker* tracker) const {
// broken into the upper/lower 32 bits to be converted back in JS,
// because there is no Uint64Array in JS.
// The third entry contains the remaining nanosecond part of the value.
void BindingData::NumberImpl(BindingData* receiver) {
void BindingData::HrtimeImpl(BindingData* receiver) {
uint64_t t = uv_hrtime();
receiver->hrtime_buffer_[0] = (t / NANOS_PER_SEC) >> 32;
receiver->hrtime_buffer_[1] = (t / NANOS_PER_SEC) & 0xffffffff;
receiver->hrtime_buffer_[2] = t % NANOS_PER_SEC;
}

void BindingData::BigIntImpl(BindingData* receiver) {
void BindingData::HrtimeBigIntImpl(BindingData* receiver) {
uint64_t t = uv_hrtime();
// The buffer is a Uint32Array, so we need to reinterpret it as a
// Uint64Array to write the value. The buffer is valid at this scope so we
Expand All @@ -706,12 +706,12 @@ void BindingData::BigIntImpl(BindingData* receiver) {
fields[0] = t;
}

void BindingData::SlowBigInt(const FunctionCallbackInfo<Value>& args) {
BigIntImpl(FromJSObject<BindingData>(args.This()));
void BindingData::SlowHrtimeBigInt(const FunctionCallbackInfo<Value>& args) {
HrtimeBigIntImpl(FromJSObject<BindingData>(args.This()));
}

void BindingData::SlowNumber(const v8::FunctionCallbackInfo<v8::Value>& args) {
NumberImpl(FromJSObject<BindingData>(args.This()));
void BindingData::SlowHrtime(const FunctionCallbackInfo<Value>& args) {
HrtimeImpl(FromJSObject<BindingData>(args.This()));
}

bool BindingData::PrepareForSerialization(Local<Context> context,
Expand Down
1 change: 0 additions & 1 deletion src/node_wasi.cc
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ inline void EinvalError() {}

template <typename FT, FT F, typename R, typename... Args>
R WASI::WasiFunction<FT, F, R, Args...>::FastCallback(
Local<Object> unused,
Local<Object> receiver,
Args... args,
// NOLINTNEXTLINE(runtime/references) This is V8 api.
Expand Down
3 changes: 1 addition & 2 deletions src/node_wasi.h
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,7 @@ class WASI : public BaseObject,
v8::Local<v8::FunctionTemplate>);

private:
static R FastCallback(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
static R FastCallback(v8::Local<v8::Object> receiver,
Args...,
v8::FastApiCallbackOptions&);

Expand Down
15 changes: 6 additions & 9 deletions src/timers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,8 @@ void BindingData::SlowScheduleTimer(const FunctionCallbackInfo<Value>& args) {
}
}

void BindingData::FastScheduleTimer(Local<Object> unused,
Local<Object> receiver,
int64_t duration) {
void BindingData::FastScheduleTimer(Local<Object> receiver, int64_t duration) {
TRACK_V8_FAST_API_CALL("timers.scheduleTimer");
ScheduleTimerImpl(FromJSObject<BindingData>(receiver), duration);
}

Expand All @@ -69,9 +68,8 @@ void BindingData::SlowToggleTimerRef(
args[0]->IsTrue());
}

void BindingData::FastToggleTimerRef(Local<Object> unused,
Local<Object> receiver,
bool ref) {
void BindingData::FastToggleTimerRef(Local<Object> receiver, bool ref) {
TRACK_V8_FAST_API_CALL("timers.toggleTimerRef");
ToggleTimerRefImpl(FromJSObject<BindingData>(receiver), ref);
}

Expand All @@ -85,9 +83,8 @@ void BindingData::SlowToggleImmediateRef(
args[0]->IsTrue());
}

void BindingData::FastToggleImmediateRef(Local<Object> unused,
Local<Object> receiver,
bool ref) {
void BindingData::FastToggleImmediateRef(Local<Object> receiver, bool ref) {
TRACK_V8_FAST_API_CALL("timers.toggleImmediateRef");
ToggleImmediateRefImpl(FromJSObject<BindingData>(receiver), ref);
}

Expand Down
11 changes: 3 additions & 8 deletions src/timers.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,23 +31,18 @@ class BindingData : public SnapshotableObject {

static void SlowScheduleTimer(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastScheduleTimer(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
static void FastScheduleTimer(v8::Local<v8::Object> receiver,
int64_t duration);
static void ScheduleTimerImpl(BindingData* data, int64_t duration);

static void SlowToggleTimerRef(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastToggleTimerRef(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
bool ref);
static void FastToggleTimerRef(v8::Local<v8::Object> receiver, bool ref);
static void ToggleTimerRefImpl(BindingData* data, bool ref);

static void SlowToggleImmediateRef(
const v8::FunctionCallbackInfo<v8::Value>& args);
static void FastToggleImmediateRef(v8::Local<v8::Object> unused,
v8::Local<v8::Object> receiver,
bool ref);
static void FastToggleImmediateRef(v8::Local<v8::Object> receiver, bool ref);
static void ToggleImmediateRefImpl(BindingData* data, bool ref);

static void CreatePerIsolateProperties(IsolateData* isolate_data,
Expand Down
Loading
Loading