[Lldb-commits] [lldb] [lldb] Fix crash in BreakpointSite::BumpHitCounts (PR #166876)

[via lldb-commits]

jimingham wrote:

Since the mutex is not recursive, that means while iterating over the locations in a collection, you can't recursively iterate over the same collection.  For that to be a problem, you'd have to go from one of the locations you were iterating over in the Collection and find your way back to the same collection.  

The Collections aren't how the Breakpoints store their locations.  So you wouldn't be able to re-lookup your current collection from the Breakpoint.  The other fairly public BreakpointLocationCollection is the "owners" BreakpointLocationCollection held by a BreakpointSite.  Fortunately, the BreakpointSite never hands out its BreakpointLocationCollection, it just uses it internally.  Something like BreakpointLocationCollection::ShouldStop does enough work that in the course of that someone might want to look up the Site and do IsBreakpointAtThisSite.  

We're saved from that danger because BreakpointSite::ShouldStop always makes a copy of the BreakpointLocationCollection to iterate over, so that would be a different lock.  This all feels a little too hand-balanced to me, and hard to ensure it won't cause deadlocks.

But I can't think of a great solution to this risk short of making this a recursive mutex, which always feels like giving up...

https://github.com/llvm/llvm-project/pull/166876