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
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
