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

src: make ReqWrap weak #44074

Merged
merged 1 commit into from Sep 14, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 0 additions & 4 deletions lib/fs.js
Expand Up @@ -24,10 +24,6 @@

'use strict';

// When using FSReqCallback, make sure to create the object only *after* all
// parameter validation has happened, so that the objects are not kept in memory
// in case they are created but never used due to an exception.
Copy link
Member

Choose a reason for hiding this comment

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

💙 It’s awesome that this caveat is going away :)


const {
ArrayPrototypePush,
BigIntPrototypeToString,
Expand Down
4 changes: 2 additions & 2 deletions src/cares_wrap.cc
Expand Up @@ -1429,7 +1429,7 @@ static void Query(const FunctionCallbackInfo<Value>& args) {

void AfterGetAddrInfo(uv_getaddrinfo_t* req, int status, struct addrinfo* res) {
auto cleanup = OnScopeLeave([&]() { uv_freeaddrinfo(res); });
std::unique_ptr<GetAddrInfoReqWrap> req_wrap {
BaseObjectPtr<GetAddrInfoReqWrap> req_wrap{
static_cast<GetAddrInfoReqWrap*>(req->data)};
Environment* env = req_wrap->env();

Expand Down Expand Up @@ -1502,7 +1502,7 @@ void AfterGetNameInfo(uv_getnameinfo_t* req,
int status,
const char* hostname,
const char* service) {
std::unique_ptr<GetNameInfoReqWrap> req_wrap {
BaseObjectPtr<GetNameInfoReqWrap> req_wrap{
static_cast<GetNameInfoReqWrap*>(req->data)};
Environment* env = req_wrap->env();

Expand Down
5 changes: 2 additions & 3 deletions src/connection_wrap.cc
Expand Up @@ -77,9 +77,8 @@ void ConnectionWrap<WrapType, UVType>::OnConnection(uv_stream_t* handle,
template <typename WrapType, typename UVType>
void ConnectionWrap<WrapType, UVType>::AfterConnect(uv_connect_t* req,
int status) {
std::unique_ptr<ConnectWrap> req_wrap
(static_cast<ConnectWrap*>(req->data));
CHECK_NOT_NULL(req_wrap);
BaseObjectPtr<ConnectWrap> req_wrap{static_cast<ConnectWrap*>(req->data)};
CHECK(req_wrap);
WrapType* wrap = static_cast<WrapType*>(req->handle->data);
CHECK_EQ(req_wrap->env(), wrap->env());
Environment* env = wrap->env();
Expand Down
4 changes: 2 additions & 2 deletions src/node_file.cc
Expand Up @@ -460,8 +460,8 @@ MaybeLocal<Promise> FileHandle::ClosePromise() {

CloseReq* req = new CloseReq(env(), close_req_obj, promise, object());
auto AfterClose = uv_fs_callback_t{[](uv_fs_t* req) {
std::unique_ptr<CloseReq> close(CloseReq::from_req(req));
CHECK_NOT_NULL(close);
BaseObjectPtr<CloseReq> close(CloseReq::from_req(req));
CHECK(close);
close->file_handle()->AfterClose();
if (!close->env()->can_call_into_js()) return;
Isolate* isolate = close->env()->isolate();
Expand Down
13 changes: 7 additions & 6 deletions src/req_wrap-inl.h
Expand Up @@ -20,13 +20,12 @@ ReqWrap<T>::ReqWrap(Environment* env,
AsyncWrap::ProviderType provider)
: AsyncWrap(env, object, provider),
ReqWrapBase(env) {
MakeWeak();
Reset();
}

template <typename T>
ReqWrap<T>::~ReqWrap() {
CHECK_EQ(false, persistent().IsEmpty());
}
ReqWrap<T>::~ReqWrap() {}

template <typename T>
void ReqWrap<T>::Dispatched() {
Expand Down Expand Up @@ -120,7 +119,8 @@ struct MakeLibuvRequestCallback<ReqT, void(*)(ReqT*, Args...)> {
using F = void(*)(ReqT* req, Args... args);

static void Wrapper(ReqT* req, Args... args) {
ReqWrap<ReqT>* req_wrap = ReqWrap<ReqT>::from_req(req);
BaseObjectPtr<ReqWrap<ReqT>> req_wrap{ReqWrap<ReqT>::from_req(req)};
req_wrap->Detach();
req_wrap->env()->DecreaseWaitingRequestCounter();
F original_callback = reinterpret_cast<F>(req_wrap->original_callback_);
original_callback(req, args...);
Expand All @@ -138,7 +138,6 @@ template <typename T>
template <typename LibuvFunction, typename... Args>
int ReqWrap<T>::Dispatch(LibuvFunction fn, Args... args) {
Dispatched();

// This expands as:
//
// int err = fn(env()->event_loop(), req(), arg1, arg2, Wrapper, arg3, ...)
Expand All @@ -158,8 +157,10 @@ int ReqWrap<T>::Dispatch(LibuvFunction fn, Args... args) {
env()->event_loop(),
req(),
MakeLibuvRequestCallback<T, Args>::For(this, args)...);
if (err >= 0)
if (err >= 0) {
ClearWeak();
env()->IncreaseWaitingRequestCounter();
}
return err;
}

Expand Down
1 change: 1 addition & 0 deletions src/tcp_wrap.cc
Expand Up @@ -344,6 +344,7 @@ void TCPWrap::Connect(const FunctionCallbackInfo<Value>& args,
if (err) {
delete req_wrap;
} else {
CHECK(args[2]->Uint32Value(env->context()).IsJust());
int port = args[2]->Uint32Value(env->context()).FromJust();
TRACE_EVENT_NESTABLE_ASYNC_BEGIN2(TRACING_CATEGORY_NODE2(net, native),
"connect",
Expand Down
2 changes: 1 addition & 1 deletion src/udp_wrap.cc
Expand Up @@ -669,7 +669,7 @@ int UDPWrap::RecvStop() {


void UDPWrap::OnSendDone(ReqWrap<uv_udp_send_t>* req, int status) {
std::unique_ptr<SendWrap> req_wrap{static_cast<SendWrap*>(req)};
BaseObjectPtr<SendWrap> req_wrap{static_cast<SendWrap*>(req)};
if (req_wrap->have_callback()) {
Environment* env = req_wrap->env();
HandleScope handle_scope(env->isolate());
Expand Down