llunak marked an inline comment as done.
llunak added inline comments.
================
Comment at: lldb/source/Core/Debugger.cpp:1969
+llvm::ThreadPool &Debugger::GetThreadPool() {
+ static llvm::ThreadPool threadpool(llvm::optimal_concurrency());
+ return threadpool;
----------------
clayborg wrote:
> 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;
> ```
>
I've changed the code to initialize/cleanup from Debugger ctor/dtor.
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
