On Wed, 8 May 2024 20:34:01 GMT, Chris Plummer <cjplum...@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
