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