[Lldb-commits] [lldb] Convert ThreadPlanStack's mutex to a shared mutex. (PR #116438)

[via lldb-commits]

jimingham wrote:

The ThreadPlans themselves don't lock access to their state as it stands now.  That's not really a problem because ThreadPlans only mutate their state in the course of the ThreadList::ShouldStop process, which happens before the new ThreadPlanStack is accessible to other parts of lldb - since we aren't formally stopped yet.  By the time general code is accessing them, they are just reporting already computed data.

Note if concurrent access to ThreadPlans were a problem, this change wouldn't substantially affect that.  We don't delegate the tasks that do the majority of the work - like `ThreadPlan::ShouldStop` - to the ThreadPlanStack, we find elements of the stack, and query them directly.  

I'm not sure how to express this more formally.  We'd have to have a couple of domains for handling ThreadPlans, one that happens serially before anything but the ShouldStop mechanism sees the newly stopped Thread, and one that happens when generic code is asking questions.  The former domain is a "writer domain" and the latter a "reader domain".  

This is currently enforced informally because the "writer domain" functions aren't useful once you've stopped, and the things the thread plans store don't get computed lazily.

Also, all the changes where I substituted the recursive_mutex with the Writer lock are protected the same way they were before, so we only need to look at the ones that have the Reader lock. The only ones of those that might do work in the ThreadPlans are:

ThreadPlanStack::GetReturnValueObject
ThreadPlanStack::GetExpressionVariable
ThreadPlanStack::ClearThreadCache

In practice those three shouldn't be a problem.  In all the extant uses, the first two aren't actually modifying, they always read out a value that was cached previously.  So those are really reader operations.  ClearThreadCache should only happen as part of preparing for a stop, before we've made any Threads (and so any ThreadPlanStack's) available to general-purpose code.  That's an example of a call that really only makes sense as part of ShouldStop.

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