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

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

Reply via email to