[Lldb-commits] [PATCH] D126240: [lldb] Tighten the scope of a couple of locks in DataFormatters.

Sat Jun 4 03:01:43 PDT 2022

hawkinsw added inline comments.

================
Comment at: lldb/source/DataFormatters/FormatManager.cpp:595
+  {
+    std::lock_guard<std::recursive_mutex> guard(m_language_categories_mutex);
+    m_language_categories_map[lang_type] =
----------------
jgorbe wrote:
> hawkinsw wrote:
> > Forgive me if I am speaking out of turn, but do we need to check again here for whether `lang_type` is in the map? In other words, it seems possible that (in some zany situation) while we are doing the memory allocation (line 593) someone else has come along and added that category since we released the lock on it in 592. 
> > 
> > I hope that this is helpful. Please (again) forgive me if I am speaking out of turn.
> Not at all! Thank you for your comment! :)
> 
> You are absolutely right, I had overlooked this case. Another annoying consequence is that in this case the racing threads would create multiple `LanguageCategory` objects. The `LanguageCategory` constructor calls `Enable` which in turn ends up calling the `Changed` method of the listener, so even if we check the map before insertion I think we'd still see the listener receiving multiple notifications that the category was enabled, which is not ideal.
> 
> I don't know how to properly fix this. I'll give it some more thought.
Glad that it was helpful! I will keep an eye on this PR and be ready to help again if you can find a way out of that tricky conundrum!! :-)

Will

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126240