RFR: 8244679: JVM/TI GetCurrentContendedMonitor/contmon001 failed due to "(IsSameObject#3) unexpected monitor object: 0x000000562336DBA8" [v2]

[David Holmes]

On Thu, 12 Nov 2020 08:11:10 GMT, Robbin Ehn <rehn at openjdk.org> wrote:

>> As the stack trace in the bug shows, we cannot load classes, since we may take a monitor.
>> Resulting in an unexpected result to GetCurrentContendedMonitor().
>> Trying to use some decent primitive, e.g. Wicket/Semaphore/.., without being implementation dependent means we must warm up every possible scenario, since it may use a new class.
>> Instead I here just use sleep + volatile for the barriers.
>> 
>> I cannot reproduce with these changes.
>> 
>> Chewing through T6 as most issues have been seen there - passed.
>
> Robbin Ehn has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Fixed comment

Hi Robbin,

Please see the bug report for more discussion.

Bottom line: I now agree this is the right kind of fix for this situation. I could nit pick on the variable naming but lets just get this done.

Thanks for your patience on this. It is important to fully understand how these situations can arise.

David

test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetCurrentContendedMonitor/contmon001.java line 67:

> 65:     public static int run(String argv[], PrintStream ref) {
> 66:         out = ref;
> 67:         doSleep(); // If we need to load any classes to execute doSleep(), do it now.

Well intentioned but not really useful. The classes used on the normal execution path are already loaded during VM initialization. The exceptional paths can still lead to class loading/linking/synchronization, so this preemptive call doesn't help that case anyway.

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

Marked as reviewed by dholmes (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/1177