src: make additional cleanups in node locks impl

* Track memory held by the Lock instance
* Clean up some Utf8/TwoByteString handling

PR-URL: #60061
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>

Author: jasnell

src/node_locks.cc

@@ -23,12 +23,10 @@ using v8::Isolate;
 using v8::Local;
 using v8::LocalVector;
 using v8::MaybeLocal;
-using v8::NewStringType;
 using v8::Object;
 using v8::ObjectTemplate;
 using v8::Promise;
 using v8::PropertyAttribute;
-using v8::String;
 using v8::Value;
 
 // Reject two promises and return `false` on failure.
@@ -51,19 +49,26 @@ Lock::Lock(Environment* env,
   released_promise_.Reset(env_->isolate(), released);
 }
 
+void Lock::MemoryInfo(node::MemoryTracker* tracker) const {
+  tracker->TrackFieldWithSize("name", name_.size());
+  tracker->TrackField("client_id", client_id_);
+  tracker->TrackField("waiting_promise", waiting_promise_);
+  tracker->TrackField("released_promise", released_promise_);
+}
+
 LockRequest::LockRequest(Environment* env,
                          Local<Promise::Resolver> waiting,
                          Local<Promise::Resolver> released,
                          Local<Function> callback,
                          const std::u16string& name,
                          Lock::Mode mode,
-                         const std::string& client_id,
+                         std::string client_id,
                          bool steal,
                          bool if_available)
     : env_(env),
       name_(name),
       mode_(mode),
-      client_id_(client_id),
+      client_id_(std::move(client_id)),
       steal_(steal),
       if_available_(if_available) {
   waiting_promise_.Reset(env_->isolate(), waiting);
@@ -85,6 +90,7 @@ Local<DictionaryTemplate> GetLockInfoTemplate(Environment* env) {
   return tmpl;
 }
 
+// The request here can be either a Lock or a LockRequest.
 static MaybeLocal<Object> CreateLockInfoObject(Environment* env,
                                                const auto& request) {
   auto tmpl = GetLockInfoTemplate(env);
@@ -573,22 +579,13 @@ void LockManager::Request(const FunctionCallbackInfo<Value>& args) {
   CHECK(args[4]->IsBoolean());   // ifAvailable
   CHECK(args[5]->IsFunction());  // callback
 
-  Local<String> resource_name_str = args[0].As<String>();
-  TwoByteValue resource_name_utf16(isolate, resource_name_str);
-  std::u16string resource_name(
-      reinterpret_cast<const char16_t*>(*resource_name_utf16),
-      resource_name_utf16.length());
-  String::Utf8Value client_id_utf8(isolate, args[1]);
-  std::string client_id(*client_id_utf8);
-  String::Utf8Value mode_utf8(isolate, args[2]);
-  std::string mode_str(*mode_utf8);
+  TwoByteValue resource_name(isolate, args[0]);
+  Utf8Value client_id(isolate, args[1]);
+  Utf8Value mode(isolate, args[2]);
   bool steal = args[3]->BooleanValue(isolate);
   bool if_available = args[4]->BooleanValue(isolate);
   Local<Function> callback = args[5].As<Function>();
 
-  Lock::Mode lock_mode =
-      mode_str == "shared" ? Lock::Mode::Shared : Lock::Mode::Exclusive;
-
   Local<Promise::Resolver> waiting_promise;
   Local<Promise::Resolver> released_promise;
 
@@ -611,15 +608,17 @@ void LockManager::Request(const FunctionCallbackInfo<Value>& args) {
       env->AddCleanupHook(LockManager::OnEnvironmentCleanup, env);
     }
 
-    auto lock_request = std::make_unique<LockRequest>(env,
-                                                      waiting_promise,
-                                                      released_promise,
-                                                      callback,
-                                                      resource_name,
-                                                      lock_mode,
-                                                      client_id,
-                                                      steal,
-                                                      if_available);
+    auto lock_request = std::make_unique<LockRequest>(
+        env,
+        waiting_promise,
+        released_promise,
+        callback,
+        resource_name.ToU16String(),
+        mode.ToStringView() == "shared" ? Lock::Mode::Shared
+                                        : Lock::Mode::Exclusive,
+        client_id.ToString(),
+        steal,
+        if_available);
     // Steal requests get priority by going to front of queue
     if (steal) {
       manager->pending_queue_.emplace_front(std::move(lock_request));
@@ -842,6 +841,10 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(OnIfAvailableReject);
 }
 
+void LockHolder::MemoryInfo(node::MemoryTracker* tracker) const {
+  tracker->TrackField("lock", lock_);
+}
+
 BaseObjectPtr<LockHolder> LockHolder::Create(Environment* env,
                                              std::shared_ptr<Lock> lock) {
   Local<Object> obj;

src/node_locks.h

@@ -15,7 +15,7 @@
 
 namespace node::worker::locks {
 
-class Lock final {
+class Lock final : public MemoryRetainer {
  public:
   enum class Mode { Shared, Exclusive };
 
@@ -53,6 +53,10 @@ class Lock final {
     return released_promise_.Get(env_->isolate());
   }
 
+  void MemoryInfo(node::MemoryTracker* tracker) const override;
+  SET_MEMORY_INFO_NAME(Lock)
+  SET_SELF_SIZE(Lock)
+
  private:
   Environment* env_;
   std::u16string name_;
@@ -79,7 +83,7 @@ class LockHolder final : public BaseObject {
 
   std::shared_ptr<Lock> lock() const { return lock_; }
 
-  SET_NO_MEMORY_INFO()
+  void MemoryInfo(node::MemoryTracker* tracker) const override;
   SET_MEMORY_INFO_NAME(LockHolder)
   SET_SELF_SIZE(LockHolder)
 
@@ -101,7 +105,7 @@ class LockRequest final {
               v8::Local<v8::Function> callback,
               const std::u16string& name,
               Lock::Mode mode,
-              const std::string& client_id,
+              std::string client_id,
               bool steal,
               bool if_available);
   ~LockRequest() = default;

src/util.h

@@ -562,6 +562,15 @@ class Utf8Value : public MaybeStackBuffer<char> {
 class TwoByteValue : public MaybeStackBuffer<uint16_t> {
  public:
   explicit TwoByteValue(v8::Isolate* isolate, v8::Local<v8::Value> value);
+
+  inline std::u16string ToU16String() const {
+    return std::u16string(reinterpret_cast<const char16_t*>(out()), length());
+  }
+
+  inline std::u16string_view ToU16StringView() const {
+    return std::u16string_view(reinterpret_cast<const char16_t*>(out()),
+                               length());
+  }
 };
 
 class BufferValue : public MaybeStackBuffer<char> {