RFR: 8141591 : javax/management/remote/mandatory/threads/ExecutorTest.java fails intermittently

[Alexander Kulyakhtin]

Based on the discussion in the group and in the CR comments I'm inclined to think this is not a test issue, as was initially treated but a JDK issue. The proper solution is still being discussed. 

I would suggest treating the issue as a JDK bug, possibly reassigning to an engineer familiar with this part of the JDK code. 

Best regards, 
Alexander 

----- Original Message ----- 
From: shanliang.jiang at oracle.com 
To: alexander.kulyakhtin at oracle.com 
Cc: jaroslav.bachorik at oracle.com, martinrb at google.com, serviceability-dev at openjdk.java.net 
Sent: Tuesday, November 10, 2015 3:35:06 PM GMT +03:00 Iraq 
Subject: Re: RFR: 8141591 : javax/management/remote/mandatory/threads/ExecutorTest.java fails intermittently 


When the executor is shutdown, the client and server were closed, so the test should not receive any exception. 


The better solution is fix in com.sun.jmx.remote.internal.ClientNotifForwarder Line 829: 
synchronized (ClientNotifForwarder.this) { 
if (state == STARTED) { 
executor.execute(new NotifFetcher()); 
} 
} 


Thanks to work on this issue. 
Shanliang 





On 10 Nov 2015, at 12:36, Alexander Kulyakhtin < alexander.kulyakhtin at oracle.com > wrote: 

Hi Jaroslav, Shanliang Jiang 

Thank you very much for the review. 

Following the comments from Shanliang Jiang on https://bugs.openjdk.java.net/browse/JDK-8141591 could it be possible to make, instead, a fix in the jdk source and leave the test unchanged 

as per the following webrev : http://cr.openjdk.java.net/~akulyakh/814591jdk/src/java.management/share/classes/com/sun/jmx/remote/internal/ClientNotifForwarder.java.udiff.html 

If these changes are problematic I will go for the test changes which you have already accepted. 

Best regards, 
Alexander 

----- Original Message ----- 
From: jaroslav.bachorik at oracle.com 
To: alexander.kulyakhtin at oracle.com , serviceability-dev at openjdk.java.net 
Cc: martinrb at google.com 
Sent: Tuesday, November 10, 2015 2:10:48 PM GMT +03:00 Iraq 
Subject: Re: RFR: 8141591 : javax/management/remote/mandatory/threads/ExecutorTest.java fails intermittently 

On 9.11.2015 19:07, Alexander Kulyakhtin wrote: 


Hi, 

Following the comments from Jaroslav and Martin I've changed the test 
to use the unbound CountDownLatch.await() 

Since await() will wait undefinitely and thus the test, if fails, will 
now fail by timeout only the code has been additionally simplified to 
reflect that. 

Please, find the updated webrev at: 

CR: https://bugs.openjdk.java.net/browse/JDK-8141591 
RFR: 
http://cr.openjdk.java.net/~akulyakh/8141591_03/test/javax/management/remote/mandatory/threads/ExecutorTest.java.udiff.html 

Looks good! 

-JB- 




Best regards, 
Alexander 

----- Original Message ----- 
From: martinrb at google.com 
To: jaroslav.bachorik at oracle.com 
Cc: alexander.kulyakhtin at oracle.com , serviceability-dev at openjdk.java.net 
Sent: Monday, November 9, 2015 7:53:08 PM GMT +03:00 Iraq 
Subject: Re: RFR: 8141591 : 
javax/management/remote/mandatory/threads/ExecutorTest.java fails 
intermittently 

The traditional way is to have all the worker tasks count down a latch 
when they're started and have the master thread wait on that before 
proceeding. 

You can use test.timeout.factor to do a scaled timed wait. 

On Mon, Nov 9, 2015 at 8:03 AM, Jaroslav Bachorik 
< jaroslav.bachorik at oracle.com < mailto:jaroslav.bachorik at oracle.com >> wrote: 

Hi Alexander, 

On 9.11.2015 16:40, Alexander Kulyakhtin wrote: 

Hi, 

Could you, please, review this test-only fix: 

CR: https://bugs.openjdk.java.net/browse/JDK-8141591 
Webrev: 
http://cr.openjdk.java.net/~akulyakh/8141591_01/test/javax/management/remote/mandatory/threads/ExecutorTest.java.udiff.html 

Before the fix, it was possible that we shut down the executor 
before all the jobs have been submitted. This resulted in 
RejectedExecutionException. 
We are modifying the test to wait on CountDownLatch untill all 
the jobs have completed their execute() 


On L175 you should be using the unbound version of await(). It would 
be better to let the harness deal with the timeout. 

Otherwise it looks good. 

Cheers, 

-JB- 


Best regards, 
Alexander 





-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/serviceability-dev/attachments/20151110/84813556/attachment.html>