> On Jun 21, 2018, at 8:03 AM, Pavel Labath via Phabricator > <revi...@reviews.llvm.org> wrote: > > labath added a comment. > > In https://reviews.llvm.org/D48393#1139270, @clayborg wrote: > >> In https://reviews.llvm.org/D48393#1138989, @labath wrote: >> >>> I am not sure this will actually solve the problems you are seeing. This >>> may avoid corrupting the internal DenseMap data structures, but it will not >>> make the algorithm using them actually correct. >>> For example the pattern in `ParseTypeFromDWARF` is: >>> >>> 1. check the "already parsed map". If the DIE is already parsed then you're >>> done. >>> 2. if the map contains the magic "DIE_IS_BEING_PARSED" key, abort >>> (recursive dwarf references) >>> 3. otherwise, insert the "DIE_IS_BEING_PARSED" key into the map >>> 4. do the parsing, which potentially involves recursive >>> `ParseTypeFromDWARF` calls >>> 5. insert the parsed type into the map >>> >>> What you do is make each of the steps (1), (3), (5) atomic individually. >>> However, the whole algorithm is not correct unless the whole sequence is >>> atomic as a whole. Otherwise, if you have two threads trying to parse the >>> same DIE (directly or indirectly), one of them could see the intermediate >>> DIE_IS_BEING_PARSED and incorrectly assume that it encountered recursive >>> types. >> >> >> We need to make #1 atomic. >> #2 would need to somehow know if the type is already being parsed >> recursively by the current thread. If so, then do what we do now. If not, we >> need a way to wait on the completion of this type so the other parsing >> thread can complete it and put it into the map, at which time we grab the >> right value from the map >> So #6 step would need to be added so after we do put it into the map, we can >> notify other threads that might be waiting > > > It's even more complicated than that, in case you really have reference > cycles, you can have multiple threads starting parsing from different points > in that cycle, and getting deadlocked waiting for the DIE_IS_BEING_PARSED > results from each other. > > 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.
Doh, that sounds like what I just tried to describe in my other mail. I should read threads completely before replying. Fred _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
