On Thu, 9 May 2024 12:32:12 GMT, Serguei Spitsyn <sspit...@openjdk.org> wrote:

>> But 2 and 1 are very different. You can call them both leaf violations, but 
>> they are leaf violations for very different reasons, and 2 is more akin to a 
>> rank violation than a leaf violation.
>> 
>> I'm reversing the ranks and reworking the loop a bit (both the comments and 
>> how the errors are reported). I'll try to post later tonight after testing 
>> is done.
>
> This is updated function `assertOrderFailure()` to print "RANK ORDER" failure 
> for the case #2 above:
> 
> static void
> assertOrderFailure(jthread thread,  DebugRawMonitorRank rank, 
> DebugRawMonitor* dbg_monitor, char* msg)
> {
>     char* threadName = getThreadName(thread);
>      tty_message("DebugRawMonitor %s failure: (%s: %d > %d) for thread (%s)",
>                  msg, dbg_monitor->name, dbg_monitor->rank, rank, threadName);
> 
>      if (rank < FIRST_LEAF_DEBUG_RAW_MONITOR &&
>          dbg_monitor ->rank >= FIRST_LEAF_DEBUG_RAW_MONITOR) {
>          tty_message("DebugRawMonitor RANK ORDER failure: (%s: %d > %d) for 
> thread (%s)",
>                      msg, dbg_monitor->name, dbg_monitor->rank, rank, 
> threadName);
>      }
>      jvmtiDeallocate(threadName);
>      JDI_ASSERT(JNI_FALSE);
> }

I've given some more thought to this "leaf monitor" concept and decided it's 
not worth having around. The only thing that results in branding a monitor as a 
leaf monitor is the fact that no additional monitors are locked while the leaf 
monitor is held. This is not actually a requirement, but simply an observation 
made for these monitors. If any additional locking was introduced while holding 
a leaf monitor, that is not necessarily a bad thing. The list of leaf monitors 
is simply what was left over at the bottom of the rankings after all monitors 
that needed a higher rank were given one (and this was the result of an 
iterative process of running tests and seeing what asserts were triggered 
because of misranking some monitors). This list started out long, but after 
fixing all the rank issues we ended up with just 4.

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

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

Reply via email to