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

[Alexander Kulyakhtin]

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
>
>
>