vsk created this revision.
vsk added reviewers: teemperor, JDevlieghere, jingham.
Herald added a project: LLDB.
vsk requested review of this revision.

AFAICT, ~TargetList simply implements the default destructor, plus some
locking.

The history is murky, so I'm not sure why we do this locking. Perhaps,
at some point, it was possible to delete the same TargetList instance
from two different threads, setting up a race. If that were true, then
the locking would protect against the race.

Since TargetList is uniquely owned by Debugger (m_target_list), no such
race is possible today.

Testing: check-lldb


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D90895

Files:
  lldb/include/lldb/Target/TargetList.h
  lldb/source/Target/TargetList.cpp


Index: lldb/source/Target/TargetList.cpp
===================================================================
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -42,12 +42,6 @@
   CheckInWithManager();
 }
 
-// Destructor
-TargetList::~TargetList() {
-  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
-  m_target_list.clear();
-}
-
 Status TargetList::CreateTarget(Debugger &debugger,
                                 llvm::StringRef user_exe_path,
                                 llvm::StringRef triple_str,
Index: lldb/include/lldb/Target/TargetList.h
===================================================================
--- lldb/include/lldb/Target/TargetList.h
+++ lldb/include/lldb/Target/TargetList.h
@@ -42,8 +42,6 @@
     return GetStaticBroadcasterClass();
   }
 
-  ~TargetList() override;
-
   /// Create a new Target.
   ///
   /// Clients must use this function to create a Target. This allows


Index: lldb/source/Target/TargetList.cpp
===================================================================
--- lldb/source/Target/TargetList.cpp
+++ lldb/source/Target/TargetList.cpp
@@ -42,12 +42,6 @@
   CheckInWithManager();
 }
 
-// Destructor
-TargetList::~TargetList() {
-  std::lock_guard<std::recursive_mutex> guard(m_target_list_mutex);
-  m_target_list.clear();
-}
-
 Status TargetList::CreateTarget(Debugger &debugger,
                                 llvm::StringRef user_exe_path,
                                 llvm::StringRef triple_str,
Index: lldb/include/lldb/Target/TargetList.h
===================================================================
--- lldb/include/lldb/Target/TargetList.h
+++ lldb/include/lldb/Target/TargetList.h
@@ -42,8 +42,6 @@
     return GetStaticBroadcasterClass();
   }
 
-  ~TargetList() override;
-
   /// Create a new Target.
   ///
   /// Clients must use this function to create a Target. This allows
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
  • [Lldb-commits] [PATCH] ... Vedant Kumar via Phabricator via lldb-commits

Reply via email to