[Lldb-commits] [lldb] Convert the StackFrameList mutex to a shared mutex. (PR #117252)

[Pavel Labath via lldb-commits]

https://github.com/labath commented:

This switching of lock types is making me very nervous. Every switch is an opportunity to get preempted, which means you have to recheck any invariants before you do that -- you can't just assume that conditions you tested earlier in the function still hold.

That essentially forces you to treat all work being done under one lock type as a single unit -- with well defined inputs, outputs, pre- and post-conditions (okay, maybe it doesn't, but I'd argue that if you can't say what is the outcome of a block of code like this, then its very hard to guarantee or check that its being used correctly).

Now, if we have a piece of code with inputs and outputs and pre- and post-conditions, then the best thing to do is to put that in a function. For something like stack unwinding, my ideal setup would be to have one function for the "shared" work, one for the "exclusive" work and then one to orchestrate everything (each one can be further subdivided as needed).
```
GetFramesUpTo(idx) {
  frame = GetCachedFrame(idx); // uses read/shared lock internally
  if (!frame) {
    // uses write/exclusive lock internally, cannot assume that frame[idx] does not exist as it might have been created in the meantime.
    frame = GetOrCreateFrame(idx); 
  }
  // Do something with the frame (with or without a lock).
}
```

So, my question is, why can't the code be structured to look something like this (I know my example is too simple to work verbatim, but it's just an illustration)? If not, then why, and how is one supposed to reason about code like this (these functions are sufficiently complex without needing to worry about lock types and preemption)? I think it's totally reasonable and desirable (and expected?) to precede a patch like this with some refactor which makes the code more amenable to a different locking strategy.

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