RFR: 8328866: Add raw monitor rank support to the debug agent. [v4]

[Serguei Spitsyn]

On Wed, 8 May 2024 20:34:01 GMT, Chris Plummer <cjplummer at openjdk.org> wrote:

>>> The special popFrame check needs to go in the first loop only, so it shouldn't be a problem or add any complexity that we don't already have.
>> 
>> Sounds good.
>> 
>>> One things to resolve is if we enter a regular monitor while holding a leaf monitor, is this a "rank" failure or "leaf" failure. Currently in the code it is a "rank" failure. Your change would make it a "leaf" failure.
>> 
>> A "leaf" failure is more specific than a "rank order" failure, so it is better to report it first. Each "leaf" failure is also  a "rank order" failure (AFAICS).
>> 
>>> I'm not sure why you added the "i != rank" check with the leaf check. We should never be re-entering a leaf monitor. The same "dbg_monitor->ownerThread != NULL" check as in the first loop should be used.
>> 
>> You are right. This check is not needed and has to be removed. I was thinking a leaf monitor can be grabbed recursively.
>
>> A "leaf" failure is more specific than a "rank order" failure, so it is better to report it first. Each "leaf" failure is also a "rank order" failure (AFAICS).
> 
> It's not just a matter of which is reported first. Even if you swap the order of your two loops you get the same result. The problem is the "rank" loop does not check if any of the leaf monitors are already held. I think to fix that it would have to iterate up to NUM_DEBUG_RAW_MONITORS, which means there is overlap with the range that the "leaf" loop iterators over.

In fact, I do not understand why reporting the "rank order" violation is important what "leaf rank order" is violated. I feel that I'm missing something. Let me think on it a little bit.

-------------

PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1594725455