inspector: tie objects lifetime to the thread they belong to

PR-URL: #22242 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Aleksei Koziatinskii <[email protected]>

Author: Eugene Ostroukhov

src/inspector/main_thread_interface.cc

@@ -3,6 +3,7 @@
 #include"node_mutex.h"
 #include"v8-inspector.h"
 
+#include<functional>
 #include<unicode/unistr.h>
 
 namespacenode{
@@ -13,56 +14,72 @@ using v8_inspector::StringView;
 using v8_inspector::StringBuffer;
 
 template <typename T>
-classDeleteRequest : publicRequest{
+classDeletableWrapper : publicDeletable{
 public:
-explicitDeleteRequest(T* object) : object_(object){}
-voidCall() override{
-delete object_;
+explicitDeletableWrapper(std::unique_ptr<T> object)
+ : object_(std::move(object)){}
+~DeletableWrapper() override = default;
+
+static T* get(MainThreadInterface* thread, int id){
+return
+static_cast<DeletableWrapper<T>*>(thread->GetObject(id))->object_.get();
  }
 
 private:
-T* object_;
+std::unique_ptr<T> object_;
 };
 
-template <typename Target, typename Arg>
-classSingleArgumentFunctionCall : publicRequest{
-public:
-using Fn = void (Target::*)(Arg);
+template <typename T>
+std::unique_ptr<Deletable> WrapInDeletable(std::unique_ptr<T> object){
+return std::unique_ptr<DeletableWrapper<T>>(
+new DeletableWrapper<T>(std::move(object)));
+}
 
-SingleArgumentFunctionCall(Target* target, Fn fn, Arg argument)
- : target_(target),
-fn_(fn),
- arg_(std::move(argument)){}
+template <typename Factory>
+classCreateObjectRequest : publicRequest{
+public:
+CreateObjectRequest(int object_id, Factory factory)
+ : object_id_(object_id), factory_(std::move(factory)){}
 
-voidCall() override{
-Apply(target_, fn_, std::move(arg_));
+voidCall(MainThreadInterface* thread){
+thread->AddObject(object_id_, WrapInDeletable(factory_(thread)));
  }
 
 private:
-template <typename Element>
-voidApply(Element* target, Fn fn, Arg arg){
- (target->*fn)(std::move(arg));
+int object_id_;
+ Factory factory_;
+};
+
+template <typename Factory>
+std::unique_ptr<Request> NewCreateRequest(int object_id, Factory factory){
+return std::unique_ptr<Request>(
+new CreateObjectRequest<Factory>(object_id, std::move(factory)));
+}
+
+classDeleteRequest : publicRequest{
+public:
+explicitDeleteRequest(int object_id) : object_id_(object_id){}
+
+voidCall(MainThreadInterface* thread) override{
+ thread->RemoveObject(object_id_);
  }
 
- Target* target_;
- Fn fn_;
- Arg arg_;
+private:
+int object_id_;
 };
 
-classPostMessageRequest : publicRequest{
+template <typename Target, typename Fn>
+classCallRequest : publicRequest{
 public:
-PostMessageRequest(InspectorSessionDelegate* delegate,
- StringView message)
- : delegate_(delegate),
-message_(StringBuffer::create(message)){}
+CallRequest(int id, Fn fn) : id_(id), fn_(std::move(fn)){}
 
-voidCall() override{
-delegate_->SendMessageToFrontend(message_->string());
+voidCall(MainThreadInterface* thread) override{
+fn_(DeletableWrapper<Target>::get(thread, id_));
  }
 
 private:
-InspectorSessionDelegate* delegate_;
-std::unique_ptr<StringBuffer> message_;
+int id_;
+Fn fn_;
 };
 
 classDispatchMessagesTask : publicv8::Task{
@@ -88,45 +105,63 @@ void DisposePairCallback(uv_handle_t* ref){
 template <typename T>
 classAnotherThreadObjectReference{
 public:
-// We create it on whatever thread, just make sure it gets disposed on the
-// proper thread.
-AnotherThreadObjectReference(std::shared_ptr<MainThreadHandle> thread,
- T* object)
- : thread_(thread), object_(object){
+AnotherThreadObjectReference(
+ std::shared_ptr<MainThreadHandle> thread, int object_id)
+ : thread_(thread), object_id_(object_id){}
+
+template <typename Factory>
+AnotherThreadObjectReference(
+ std::shared_ptr<MainThreadHandle> thread, Factory factory)
+ : AnotherThreadObjectReference(thread, thread->newObjectId()){
+ thread_->Post(NewCreateRequest(object_id_, std::move(factory)));
  }
 AnotherThreadObjectReference(AnotherThreadObjectReference&) = delete;
 
 ~AnotherThreadObjectReference(){
 // Disappearing thread may cause a memory leak
-CHECK(thread_->Post(
- std::unique_ptr<DeleteRequest<T>>(new DeleteRequest<T>(object_))));
- object_ = nullptr;
+ thread_->Post(
+ std::unique_ptr<DeleteRequest>(newDeleteRequest(object_id_)));
  }
 
-template <typename Fn, typename Arg>
-voidPost(Fn fn, Arg argument) const{
-using R = SingleArgumentFunctionCall<T, Arg>
- thread_->Post(std::unique_ptr<R>(newR(object_, fn, std::move(argument))));
+template <typename Fn>
+voidCall(Fn fn) const{
+using Request = CallRequest<T, Fn>
+ thread_->Post(std::unique_ptr<Request>(
+newRequest(object_id_, std::move(fn))));
  }
 
- T* get() const{
-return object_;
+template <typename Arg>
+voidCall(void (T::*fn)(Arg), Arg argument) const{
+Call(std::bind(Apply<Arg>, std::placeholders::_1, fn, std::move(argument)));
  }
 
 private:
+// This has to use non-const reference to support std::bind with non-copyable
+// types
+template <typename Argument>
+staticvoidApply(T* target, void (T::*fn)(Argument),
+/* NOLINT (runtime/references) */ Argument& argument){
+ (target->*fn)(std::move(argument));
+ }
+
  std::shared_ptr<MainThreadHandle> thread_;
-T* object_;
+constint object_id_;
 };
 
 classMainThreadSessionState{
 public:
-MainThreadSessionState(
- std::shared_ptr<MainThreadHandle> thread,
-bool prevent_shutdown) : thread_(thread),
-prevent_shutdown_(prevent_shutdown){}
+MainThreadSessionState(MainThreadInterface* thread, bool prevent_shutdown)
+ : thread_(thread),
+prevent_shutdown_(prevent_shutdown){}
+
+static std::unique_ptr<MainThreadSessionState> Create(
+ MainThreadInterface* thread, bool prevent_shutdown){
+return std::unique_ptr<MainThreadSessionState>(
+newMainThreadSessionState(thread, prevent_shutdown));
+ }
 
 voidConnect(std::unique_ptr<InspectorSessionDelegate> delegate){
- Agent* agent = thread_->GetInspectorAgent();
+ Agent* agent = thread_->inspector_agent();
 if (agent != nullptr)
  session_ = agent->Connect(std::move(delegate), prevent_shutdown_);
  }
@@ -136,7 +171,7 @@ class MainThreadSessionState{
  }
 
 private:
-std::shared_ptr<MainThreadHandle> thread_;
+MainThreadInterface* thread_;
 bool prevent_shutdown_;
  std::unique_ptr<InspectorSession> session_;
 };
@@ -148,12 +183,14 @@ class CrossThreadInspectorSession : public InspectorSession{
  std::shared_ptr<MainThreadHandle> thread,
  std::unique_ptr<InspectorSessionDelegate> delegate,
 bool prevent_shutdown)
- : state_(thread, new MainThreadSessionState(thread, prevent_shutdown)){
- state_.Post(&MainThreadSessionState::Connect, std::move(delegate));
+ : state_(thread, std::bind(MainThreadSessionState::Create,
+ std::placeholders::_1,
+ prevent_shutdown)){
+ state_.Call(&MainThreadSessionState::Connect, std::move(delegate));
  }
 
 voidDispatch(const StringView& message) override{
- state_.Post(&MainThreadSessionState::Dispatch,
+ state_.Call(&MainThreadSessionState::Dispatch,
 StringBuffer::create(message));
  }
 
@@ -163,13 +200,15 @@ class CrossThreadInspectorSession : public InspectorSession{
 
 classThreadSafeDelegate : publicInspectorSessionDelegate{
 public:
-ThreadSafeDelegate(std::shared_ptr<MainThreadHandle> thread,
- std::unique_ptr<InspectorSessionDelegate> delegate)
- : thread_(thread), delegate_(thread, delegate.release()){}
+ThreadSafeDelegate(std::shared_ptr<MainThreadHandle> thread, int object_id)
+ : thread_(thread), delegate_(thread, object_id){}
 
 voidSendMessageToFrontend(const v8_inspector::StringView& message) override{
- thread_->Post(std::unique_ptr<Request>(
-newPostMessageRequest(delegate_.get(), message)));
+ delegate_.Call(
+ [m = StringBuffer::create(message)]
+ (InspectorSessionDelegate* delegate){
+ delegate->SendMessageToFrontend(m->string());
+ });
  }
 
 private:
@@ -252,7 +291,7 @@ void MainThreadInterface::DispatchMessages(){
  MessageQueue::value_type task;
 std::swap(dispatching_message_queue_.front(), task);
  dispatching_message_queue_.pop_front();
- task->Call();
+ task->Call(this);
  }
  } while (had_messages);
  dispatching_messages_ = false;
@@ -264,6 +303,26 @@ std::shared_ptr<MainThreadHandle> MainThreadInterface::GetHandle(){
 return handle_;
 }
 
+voidMainThreadInterface::AddObject(int id,
+ std::unique_ptr<Deletable> object){
+CHECK_NE(nullptr, object);
+ managed_objects_[id] = std::move(object);
+}
+
+voidMainThreadInterface::RemoveObject(int id){
+CHECK_EQ(1, managed_objects_.erase(id));
+}
+
+Deletable* MainThreadInterface::GetObject(int id){
+auto iterator = managed_objects_.find(id);
+// This would mean the object is requested after it was disposed, which is
+// a coding error.
+CHECK_NE(managed_objects_.end(), iterator);
+ Deletable* pointer = iterator->second.get();
+CHECK_NE(nullptr, pointer);
+return pointer;
+}
+
 std::unique_ptr<StringBuffer> Utf8ToStringView(const std::string& message){
  icu::UnicodeString utf16 = icu::UnicodeString::fromUTF8(
 icu::StringPiece(message.data(), message.length()));
@@ -303,10 +362,12 @@ Agent* MainThreadHandle::GetInspectorAgent(){
 }
 
 std::unique_ptr<InspectorSessionDelegate>
-MainThreadHandle::MakeThreadSafeDelegate(
+MainThreadHandle::MakeDelegateThreadSafe(
  std::unique_ptr<InspectorSessionDelegate> delegate){
+int id = newObjectId();
+ main_thread_->AddObject(id, WrapInDeletable(std::move(delegate)));
 return std::unique_ptr<InspectorSessionDelegate>(
-newThreadSafeDelegate(shared_from_this(), std::move(delegate)));
+newThreadSafeDelegate(shared_from_this(), id));
 }
 
 boolMainThreadHandle::Expired(){

src/inspector/main_thread_interface.h

@@ -9,10 +9,10 @@
 #include"inspector_agent.h"
 #include"node_mutex.h"
 
+#include<atomic>
 #include<deque>
 #include<memory>
 #include<unordered_map>
-#include<unordered_set>
 
 namespacev8_inspector{
 classStringBuffer;
@@ -21,31 +21,41 @@ class StringView;
 
 namespacenode{
 namespaceinspector{
+classMainThreadInterface;
+
 classRequest{
 public:
-virtualvoidCall() = 0;
+virtualvoidCall(MainThreadInterface*) = 0;
 virtual~Request(){}
 };
 
+classDeletable{
+public:
+virtual~Deletable(){}
+};
+
 std::unique_ptr<v8_inspector::StringBuffer> Utf8ToStringView(
 const std::string& message);
 
 using MessageQueue = std::deque<std::unique_ptr<Request>>
-classMainThreadInterface;
 
 classMainThreadHandle : publicstd::enable_shared_from_this<MainThreadHandle>{
 public:
 explicitMainThreadHandle(MainThreadInterface* main_thread)
- : main_thread_(main_thread){}
+ : main_thread_(main_thread){
+ }
 ~MainThreadHandle(){
 CHECK_NULL(main_thread_); // main_thread_ should have called Reset
  }
  std::unique_ptr<InspectorSession> Connect(
  std::unique_ptr<InspectorSessionDelegate> delegate,
 bool prevent_shutdown);
+intnewObjectId(){
+return ++next_object_id_;
+ }
 boolPost(std::unique_ptr<Request> request);
  Agent* GetInspectorAgent();
- std::unique_ptr<InspectorSessionDelegate> MakeThreadSafeDelegate(
+ std::unique_ptr<InspectorSessionDelegate> MakeDelegateThreadSafe(
  std::unique_ptr<InspectorSessionDelegate> delegate);
 boolExpired();
 
@@ -55,6 +65,7 @@ class MainThreadHandle : public std::enable_shared_from_this<MainThreadHandle>{
  MainThreadInterface* main_thread_;
  Mutex block_lock_;
 int next_session_id_ = 0;
+ std::atomic_int next_object_id_ ={1};
 
 friendclassMainThreadInterface;
 };
@@ -72,6 +83,9 @@ class MainThreadInterface{
  Agent* inspector_agent(){
 return agent_;
  }
+voidAddObject(int handle, std::unique_ptr<Deletable> object);
+ Deletable* GetObject(int id);
+voidRemoveObject(int handle);
 
 private:
 using AsyncAndInterface = std::pair<uv_async_t, MainThreadInterface*>
@@ -92,6 +106,7 @@ class MainThreadInterface{
  v8::Platform* const platform_;
  DeleteFnPtr<AsyncAndInterface, CloseAsync> main_thread_request_;
  std::shared_ptr<MainThreadHandle> handle_;
+ std::unordered_map<int, std::unique_ptr<Deletable>> managed_objects_;
 };
 
 } // namespace inspector