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

[David Holmes]

On 12/11/2020 8:57 am, Daniel D.Daugherty wrote:
> On Wed, 11 Nov 2020 20:33:15 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.
> 
> I like the fix. Taking Wicket out of play will help with class loading
> issues in this test. You don't really need to deal with unexpected
> locking events from the ObjectLocker usage by class loading.

The whole point of using Wicket in these tests is to try and correctly 
synchronize the threads involved so that we only inspect things when the 
target thread is in the right state. The serviceability folk have done a 
lot of test stabilisation work in that area. This change seems to be 
taking things in the wrong direction to me.

But this is a serviceability test issue and it is up to the 
serviceability folk to determine how they'd like this test fixed.

> 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 it would do any class loading, do it now.
> 
> Perhaps:
> // If we need to load any classes do execute doSleep(), do it now.

Typo: do execute -> to execute

Cheers,
David
-----

> test/hotspot/jtreg/vmTestbase/nsk/jvmti/GetCurrentContendedMonitor/contmon001.java line 79:
> 
>> 77:         contmon001a thr = new contmon001a();
>> 78:         startingBarrier = new Wicket();
>> 79:         waitingBarrier = new Wicket();
> 
> If you want to continue to use Wicket, then "all" you have to do is
> execute one back-and-forth of the Wicket before executing the
> test portion itself. That should get all the Wicket classes loaded
> before you really need them. Unless, of course, there's some class
> that isn't loaded unless there's an error or something...
> 
> However, by using volatile booleans instead, you get out of the
> game of trying to figure out how much of the Wicket code needs
> to be exercised to get everything you might need loaded.
> 
> -------------
> 
> Marked as reviewed by dcubed (Reviewer).
> 
> PR: https://git.openjdk.java.net/jdk/pull/1177
>