src: make ReqWrap weak

This commit allows throwing an exception after creating `FSReqCallback`

Co-authored-by: Anna Henningsen <[email protected]>
PR-URL: nodejs/node#44074
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Author: 2 people

lib/fs.js

@@ -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.
-
 const {
   ArrayPrototypePush,
   BigIntPrototypeToString,

src/cares_wrap.cc

@@ -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();
 
@@ -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();
 

src/connection_wrap.cc

@@ -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();

src/node_file.cc

@@ -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();

src/req_wrap-inl.h

@@ -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() {
@@ -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...);
@@ -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, ...)
@@ -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;
 }
 

src/tcp_wrap.cc

@@ -338,6 +338,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",

src/udp_wrap.cc

@@ -665,7 +665,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());