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 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
