Skip to content
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

[v12.x] backport BaseObjectPtr, SetImmediate(), Worker+report #32301

Closed
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
afa5499
src: introduce custom smart pointers for `BaseObject`s
addaleax Sep 28, 2019
e1b1267
http2: use custom BaseObject smart pointers
addaleax Sep 28, 2019
95bf451
src: use BaseObjectPtr for keeping channel alive in dns bindings
addaleax Nov 12, 2019
cb196ce
src: remove keep alive option from SetImmediate()
addaleax Nov 12, 2019
92cb961
src: remove HandleWrap instances from list once closed
addaleax Oct 11, 2019
b18531d
src: keep object alive in stream_pipe code
addaleax Nov 26, 2019
ece9faf
src: add more `can_call_into_js()` guards
addaleax Nov 26, 2019
c3fff25
src: no SetImmediate from destructor in stream_pipe code
addaleax Nov 26, 2019
2470035
src: run native immediates during Environment cleanup
addaleax Nov 26, 2019
973f8ef
src: better encapsulate native immediate list
addaleax Dec 16, 2019
5e1bcae
src: exclude C++ SetImmediate() from count
addaleax Jan 15, 2020
f1aeed5
src: add a threadsafe variant of SetImmediate()
addaleax Jan 15, 2020
3f9d1dd
src: remove AsyncRequest
addaleax Jan 15, 2020
b0ce668
src: add interrupts to Environments/Workers
addaleax Jan 16, 2020
4e1a192
src: move MemoryInfo() for worker code to .cc files
addaleax Jan 16, 2020
a9c4fe5
report: add support for Workers
addaleax Jan 16, 2020
9c43d0e
src: simplify native immediate queue running
addaleax Jan 25, 2020
1915b5b
worker: move JoinThread() back into exit callback
addaleax Jan 22, 2020
4d3275b
src: harden running native `SetImmediate()`s slightly
addaleax Jan 22, 2020
d85cb07
src: delete BaseObjectWeakPtr data when pointee is gone
addaleax Mar 20, 2020
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
21 changes: 21 additions & 0 deletions doc/api/report.md
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,7 @@ is provided below for reference.
"address": "0x000055fc7b2cb180"
}
],
"workers": [],
"environmentVariables": {
"REMOTEHOST": "REMOVED",
"MANPATH": "/opt/rh/devtoolset-3/root/usr/share/man:",
Expand Down Expand Up @@ -578,4 +579,24 @@ NODE_OPTIONS="--experimental-report --report-uncaught-exception \
Specific API documentation can be found under
[`process API documentation`][] section.

## Interaction with Workers
<!-- YAML
changes:
- version: REPLACEME
pr-url: https://github.com/nodejs/node/pull/31386
description: Workers are now included in the report.
-->

[`Worker`][] threads can create reports in the same way that the main thread
does.

Reports will include information on any Workers that are children of the current
thread as part of the `workers` section, with each Worker generating a report
in the standard report format.

The thread which is generating the report will wait for the reports from Worker
threads to finish. However, the latency for this will usually be low, as both
running JavaScript and the event loop are interrupted to generate the report.

[`process API documentation`]: process.html
[`Worker`]: worker_threads.html
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -1117,6 +1117,7 @@
'test/cctest/node_test_fixture.h',
'test/cctest/test_aliased_buffer.cc',
'test/cctest/test_base64.cc',
'test/cctest/test_base_object_ptr.cc',
'test/cctest/test_node_postmortem_metadata.cc',
'test/cctest/test_environment.cc',
'test/cctest/test_linked_binding.cc',
Expand Down
215 changes: 204 additions & 11 deletions src/base_object-inl.h
Original file line number Diff line number Diff line change
Expand Up @@ -40,30 +40,40 @@
namespace node {

BaseObject::BaseObject(Environment* env, v8::Local<v8::Object> object)
: persistent_handle_(env->isolate(), object),
env_(env) {
: persistent_handle_(env->isolate(), object), env_(env) {
CHECK_EQ(false, object.IsEmpty());
CHECK_GT(object->InternalFieldCount(), 0);
object->SetAlignedPointerInInternalField(0, static_cast<void*>(this));
env_->AddCleanupHook(DeleteMe, static_cast<void*>(this));
env->AddCleanupHook(DeleteMe, static_cast<void*>(this));
env->modify_base_object_count(1);
}

BaseObject::~BaseObject() {
RemoveCleanupHook();
env()->modify_base_object_count(-1);
env()->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));

if (UNLIKELY(has_pointer_data())) {
PointerData* metadata = pointer_data();
CHECK_EQ(metadata->strong_ptr_count, 0);
metadata->self = nullptr;
if (metadata->weak_ptr_count == 0)
delete metadata;
}

if (persistent_handle_.IsEmpty()) {
// This most likely happened because the weak callback below cleared it.
return;
}

{
v8::HandleScope handle_scope(env_->isolate());
v8::HandleScope handle_scope(env()->isolate());
object()->SetAlignedPointerInInternalField(0, nullptr);
}
}

void BaseObject::RemoveCleanupHook() {
env_->RemoveCleanupHook(DeleteMe, static_cast<void*>(this));
void BaseObject::Detach() {
CHECK_GT(pointer_data()->strong_ptr_count, 0);
pointer_data()->is_detached = true;
}

v8::Global<v8::Object>& BaseObject::persistent() {
Expand All @@ -72,14 +82,14 @@ v8::Global<v8::Object>& BaseObject::persistent() {


v8::Local<v8::Object> BaseObject::object() const {
return PersistentToLocal::Default(env_->isolate(), persistent_handle_);
return PersistentToLocal::Default(env()->isolate(), persistent_handle_);
}

v8::Local<v8::Object> BaseObject::object(v8::Isolate* isolate) const {
v8::Local<v8::Object> handle = object();

DCHECK_EQ(handle->CreationContext()->GetIsolate(), isolate);
DCHECK_EQ(env_->isolate(), isolate);
DCHECK_EQ(env()->isolate(), isolate);

return handle;
}
Expand All @@ -88,7 +98,6 @@ Environment* BaseObject::env() const {
return env_;
}


BaseObject* BaseObject::FromJSObject(v8::Local<v8::Object> obj) {
CHECK_GT(obj->InternalFieldCount(), 0);
return static_cast<BaseObject*>(obj->GetAlignedPointerFromInternalField(0));
Expand All @@ -102,20 +111,34 @@ T* BaseObject::FromJSObject(v8::Local<v8::Object> object) {


void BaseObject::MakeWeak() {
if (has_pointer_data()) {
pointer_data()->wants_weak_jsobj = true;
if (pointer_data()->strong_ptr_count > 0) return;
}

persistent_handle_.SetWeak(
this,
[](const v8::WeakCallbackInfo<BaseObject>& data) {
std::unique_ptr<BaseObject> obj(data.GetParameter());
BaseObject* obj = data.GetParameter();
// Clear the persistent handle so that ~BaseObject() doesn't attempt
// to mess with internal fields, since the JS object may have
// transitioned into an invalid state.
// Refs: https://github.com/nodejs/node/issues/18897
obj->persistent_handle_.Reset();
CHECK_IMPLIES(obj->has_pointer_data(),
obj->pointer_data()->strong_ptr_count == 0);
obj->OnGCCollect();
}, v8::WeakCallbackType::kParameter);
}

void BaseObject::OnGCCollect() {
delete this;
}

void BaseObject::ClearWeak() {
if (has_pointer_data())
pointer_data()->wants_weak_jsobj = false;

persistent_handle_.ClearWeak();
}

Expand Down Expand Up @@ -149,6 +172,176 @@ void BaseObject::InternalFieldSet(v8::Local<v8::String> property,
info.This()->SetInternalField(Field, value);
}

bool BaseObject::has_pointer_data() const {
return pointer_data_ != nullptr;
}

BaseObject::PointerData* BaseObject::pointer_data() {
if (!has_pointer_data()) {
PointerData* metadata = new PointerData();
metadata->wants_weak_jsobj = persistent_handle_.IsWeak();
metadata->self = this;
pointer_data_ = metadata;
}
CHECK(has_pointer_data());
return pointer_data_;
}

void BaseObject::decrease_refcount() {
CHECK(has_pointer_data());
PointerData* metadata = pointer_data();
CHECK_GT(metadata->strong_ptr_count, 0);
unsigned int new_refcount = --metadata->strong_ptr_count;
if (new_refcount == 0) {
if (metadata->is_detached) {
delete this;
} else if (metadata->wants_weak_jsobj && !persistent_handle_.IsEmpty()) {
MakeWeak();
}
}
}

void BaseObject::increase_refcount() {
unsigned int prev_refcount = pointer_data()->strong_ptr_count++;
if (prev_refcount == 0 && !persistent_handle_.IsEmpty())
persistent_handle_.ClearWeak();
}

template <typename T, bool kIsWeak>
BaseObject::PointerData*
BaseObjectPtrImpl<T, kIsWeak>::pointer_data() const {
if (kIsWeak) {
return data_.pointer_data;
} else {
if (get_base_object() == nullptr) return nullptr;
return get_base_object()->pointer_data();
}
}

template <typename T, bool kIsWeak>
BaseObject* BaseObjectPtrImpl<T, kIsWeak>::get_base_object() const {
if (kIsWeak) {
if (pointer_data() == nullptr) return nullptr;
return pointer_data()->self;
} else {
return data_.target;
}
}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>::~BaseObjectPtrImpl() {
if (kIsWeak) {
if (pointer_data() != nullptr &&
--pointer_data()->weak_ptr_count == 0 &&
pointer_data()->self == nullptr) {
delete pointer_data();
}
} else if (get() != nullptr) {
get()->decrease_refcount();
}
}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl() {
data_.target = nullptr;
}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(T* target)
: BaseObjectPtrImpl() {
if (target == nullptr) return;
if (kIsWeak) {
data_.pointer_data = target->pointer_data();
CHECK_NOT_NULL(pointer_data());
pointer_data()->weak_ptr_count++;
} else {
data_.target = target;
CHECK_NOT_NULL(pointer_data());
get()->increase_refcount();
}
}

template <typename T, bool kIsWeak>
template <typename U, bool kW>
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(
const BaseObjectPtrImpl<U, kW>& other)
: BaseObjectPtrImpl(other.get()) {}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(const BaseObjectPtrImpl& other)
: BaseObjectPtrImpl(other.get()) {}

template <typename T, bool kIsWeak>
template <typename U, bool kW>
BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
const BaseObjectPtrImpl<U, kW>& other) {
if (other.get() == get()) return *this;
this->~BaseObjectPtrImpl();
return *new (this) BaseObjectPtrImpl(other);
}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
const BaseObjectPtrImpl& other) {
if (other.get() == get()) return *this;
this->~BaseObjectPtrImpl();
return *new (this) BaseObjectPtrImpl(other);
}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>::BaseObjectPtrImpl(BaseObjectPtrImpl&& other)
: data_(other.data_) {
if (kIsWeak)
other.data_.target = nullptr;
else
other.data_.pointer_data = nullptr;
}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>& BaseObjectPtrImpl<T, kIsWeak>::operator=(
BaseObjectPtrImpl&& other) {
if (&other == this) return *this;
this->~BaseObjectPtrImpl();
return *new (this) BaseObjectPtrImpl(std::move(other));
}

template <typename T, bool kIsWeak>
void BaseObjectPtrImpl<T, kIsWeak>::reset(T* ptr) {
*this = BaseObjectPtrImpl(ptr);
}

template <typename T, bool kIsWeak>
T* BaseObjectPtrImpl<T, kIsWeak>::get() const {
return static_cast<T*>(get_base_object());
}

template <typename T, bool kIsWeak>
T& BaseObjectPtrImpl<T, kIsWeak>::operator*() const {
return *get();
}

template <typename T, bool kIsWeak>
T* BaseObjectPtrImpl<T, kIsWeak>::operator->() const {
return get();
}

template <typename T, bool kIsWeak>
BaseObjectPtrImpl<T, kIsWeak>::operator bool() const {
return get() != nullptr;
}

template <typename T, typename... Args>
BaseObjectPtr<T> MakeBaseObject(Args&&... args) {
return BaseObjectPtr<T>(new T(std::forward<Args>(args)...));
}

template <typename T, typename... Args>
BaseObjectPtr<T> MakeDetachedBaseObject(Args&&... args) {
BaseObjectPtr<T> target = MakeBaseObject<T>(std::forward<Args>(args)...);
target->Detach();
return target;
}

} // namespace node

#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
Expand Down
Loading