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

[Jorge Gorbe Moya via Phabricator via lldb-commits]

jgorbe 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] =
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D126240