[Lldb-commits] [PATCH] D131328: [lldb] Support fetching symbols with dsymForUUID in the background

Mon Aug 8 07:35:58 PDT 2022

JDevlieghere added a comment.

In D131328#3706202 <https://reviews.llvm.org/D131328#3706202>, @labath wrote:

> I see two problems with this patch.
>
> - it makes shutting down lldb safely impossible (because you never know if there are any unfinished tasks running). We already have problems with shutting down (grep for "intentionally leaked"), but this is guaranteed to make the problem worse. Using the global thread pool would make things better, as then one could flush the pool in SBDebugger::Terminate, or something similar

It *is* using the global thread pool, but it doesn't look like that's being flushed. I can rework that in a separate patch to make it fit the Initialize/Terminate pattern.

> - there seems to be a race between new debuggers/targets being added, and their existing instances being notified. I'm not 100% sure what will happen in this case, but it seems very easy to miss some debuggers (or targets) that were added after we took the "snapshot" of the current state

As I (tried to) explain in the comment, since we set the symbol file in the module before taking the "snapshot", I don't think this is a problem. If a target was created after, it should already know about the symbol file. Did I miss something with that approach?

> (There's also the (obvious) problem of any breakpoints that are set on code within those modules becoming effective at an unpredictable future data, but I am assuming you are aware of that.)

Yeah, but I don't see any way around that. The unpredictability is unfortunate, but from UX perspective it's not much worse than modules getting loaded at runtime (although hopefully that's more deterministic).

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

https://reviews.llvm.org/D131328