labath added a comment.

In https://reviews.llvm.org/D48393#1243195, @JDevlieghere wrote:

> In https://reviews.llvm.org/D48393#1139327, @labath wrote:
>
> > The only sane algorithm I can come up right now is to make the list of 
> > parsed dies local to each thread/parsing entity (e.g. via a "visited" 
> > list), and only update the global map once the parsing has completed 
> > (successfully or not). This can potentially duplicate some effort where one 
> > thread parses a type only to find out that it has already been parsed, but 
> > hopefully that is not going to be the common case. The alternative is some 
> > complicated resource cycle detection scheme.
>
>
> I gave this a shot in https://reviews.llvm.org/D52406 but I'm afraid it's too 
> simple to be correct. Pavel, could you give it a look and let me know whether 
> that was what you had in mind?


Yeah, I don't think this will make things fully correct. This avoids the 
problem when two threads are misinterpreting the DIE_IS_BEING_PARSED markers 
left by the other thread. However, it is still not correct when two threads are 
parsing the same DIE concurrently. Imagine the following sequence of events:

- thread A starts parsing DIE 1, checks that it still hasn't been parsed, 
inserts the DIE_IS_BEING_PARSED into the local map.
- thread B does the same
- thread A finishes parsing DIE 1. constructs a clang AST (C1) for it sets it 
as the map value
- thread B does the same overwrites the value with C2
- thread A carries on using C1
- thread B carries on using C2

This sounds like something that is not supposed to happen, though I don't 
really know what the consequences of that are. Also I am not entirely convinced 
that the mere construction of C1 and C2 does not touch some global structures 
(which would not be protected by a lock).

I agree with Greg that it would be best to restrict things such that there is 
only one instance of parsing going on at any given time for a single module. I 
think this was pretty much the state we reached when this thread fizzled out 
the last time (there are some extra emails that are not showing in phabricator 
history, the interesting ones start around here: 
http://lists.llvm.org/pipermail/lldb-commits/Week-of-Mon-20180618/041937.html). 
I think the main part that needed to be resolved is whether we need to go 
anything special when resolving debug info references *between* modules (e.g. 
to prevent A/B deadlocks).


https://reviews.llvm.org/D48393



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

Reply via email to