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

[Shanliang JIANG]

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/148a8349/attachment-0001.html>