clayborg added inline comments.

================
Comment at: lldb/source/Core/Debugger.cpp:1969
+llvm::ThreadPool &Debugger::GetThreadPool() {
+  static llvm::ThreadPool threadpool(llvm::optimal_concurrency());
+  return threadpool;
----------------
aganea wrote:
> clayborg wrote:
> > aganea wrote:
> > > Ideally this should be explicitly created on the stack & passed along on 
> > > the callstack or in a context, like MLIR does: 
> > > https://github.com/llvm/llvm-project/blob/main/mlir/include/mlir/IR/MLIRContext.h#L48
> > We need one instance of the thread pool so we can't create on the stack. 
> > 
> > Static is not great because of the C++ global destructor chain could try 
> > and use if "main" exits and we still have threads running. I would do 
> > something like the "g_debugger_list_ptr" where you create a static variable 
> > in Debugger.cpp:105 which is a pointer, and we initialize it inside of 
> > Debugger::Initialize() like we do for "g_debugger_list_ptr". Then the 
> > thread pool will not cause shutdown crashes when "main" exits before other 
> > threads.
> > 
> I meant having the `ThreadPool` in a context created by main() or the lib 
> caller, before getting here in library/plugin code, and passed along. LLD 
> does that too: 
> https://github.com/llvm/llvm-project/blob/main/lld/ELF/Driver.cpp#L94 - the 
> statics in `Debugger.cpp` seems like a workaround for that. In any case, it's 
> better than having the `static` here.
In LLDB, the lldb_private::Debugger has static functions that hand out things 
that are needed for the debugger to do its work, so I like the static function 
here. We don't allow any llvm or clang code to make it through our public API 
(in lldb::SB* classes), so we do need code inside the debugger to be able to 
access global resources and the debugger class is where this should live. 

We can also just have code like:
```
// NOTE: intentional leak to avoid issues with C++ destructor chain
static llvm::ThreadPool *g_thread_pool = nullptr;
static llvm::once_flag g_once_flag;
llvm::call_once(g_once_flag, []() {
  g_thread_pool = new llvm::ThreadPool(llvm::optimal_concurrency());
});
return *g_thread_pool;
```



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D123226/new/

https://reviews.llvm.org/D123226

_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to