[Lldb-commits] [PATCH] D40470: DWZ 07/12: Protect DWARFCompileUnit::m_die_array by a new mutex

[Jan Kratochvil via Phabricator via lldb-commits]

jankratochvil added inline comments.


================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:123
 size_t DWARFCompileUnit::ExtractDIEsIfNeeded(bool cu_die_only) {
-  const size_t initial_die_array_size = m_data->m_die_array.size();
-  if ((cu_die_only && initial_die_array_size > 0) || initial_die_array_size > 1)
-    return 0; // Already parsed
+  size_t initial_die_array_size;
+  auto already_parsed = [cu_die_only, &initial_die_array_size]() -> bool {
----------------
clayborg wrote:
> clayborg wrote:
> > I really can't tell what any of the above changed code is doing besides adding the mutex. Seems like you are trying to emulate a condition variable to halt other threads trying to get access to the DIEs. But m_die_array_finished isn't initialized in the constructor or inlined in the header file and initial_die_array_size isn't initialized so I can't really tell what this is supposed to actually do.
> This isn't initialized and it is captured and used in already_parsed below?
I am sorry but the missing initialization was just due to a wrong patch splitting.  Next patch in the series did initialize it.  Anyway it has been reworked now (after dropping whole ExtractDIEsIfNeededKind for its AllDiesButCuDieOnlyForPartialUnits).



================
Comment at: source/Plugins/SymbolFile/DWARF/DWARFCompileUnit.cpp:123-137
+  size_t initial_die_array_size;
+  auto already_parsed = [cu_die_only, &initial_die_array_size]() -> bool {
+    return (cu_die_only && initial_die_array_size > 0)
+        || initial_die_array_size > 1;
+  };
+  if (already_parsed() && m_data->m_die_array_finished)
+    return 0;
----------------
jankratochvil wrote:
> clayborg wrote:
> > clayborg wrote:
> > > I really can't tell what any of the above changed code is doing besides adding the mutex. Seems like you are trying to emulate a condition variable to halt other threads trying to get access to the DIEs. But m_die_array_finished isn't initialized in the constructor or inlined in the header file and initial_die_array_size isn't initialized so I can't really tell what this is supposed to actually do.
> > This isn't initialized and it is captured and used in already_parsed below?
> I am sorry but the missing initialization was just due to a wrong patch splitting.  Next patch in the series did initialize it.  Anyway it has been reworked now (after dropping whole ExtractDIEsIfNeededKind for its AllDiesButCuDieOnlyForPartialUnits).
> 
Condition variable is not required here I think. m_die_array_finished is initialized before any time it gets read.


https://reviews.llvm.org/D40470