jmx-dev RFR 7132590: javax/management/remote/mandatory/notif/NotificationAccessControllerTest.java fails in JDK8-B22

[Jaroslav Bachorik]

On 08/26/2014 12:08 PM, Daniel Fuchs wrote:
> Hi Jaroslav,
>
> line 143, notifs should be final, and should be of a type
> that supports concurrent access - something like
> CopyOnWriteArrayList or Collections.synchronizedList().

Taken care of. I also took the liberty to get rid off few compiler warnings.

http://cr.openjdk.java.net/~jbachorik/7132590/webrev.01

-JB-

>
> Otherwise looks good!
>
> best regards,
>
> -- daniel
>
> On 8/26/14 10:44 AM, Jaroslav Bachorik wrote:
>> On 08/21/2014 05:34 PM, shanliang wrote:
>>> Jaroslav Bachorik wrote:
>>>> On 08/21/2014 03:55 PM, shanliang wrote:
>>>>> Jaroslav,
>>>>>
>>>>> The fix should be good to fix the failure.
>>>>>
>>>>> It makes me think a special case, suppose that the test waits 2
>>>>> notifications, but the test might receive one unexpected notification
>>>>> with some more waiting, for example, with the old version, 2 expected
>>>>> notifications arrive within the first second, and the unexpected
>>>>> arrives
>>>>> in the second second, but with your fix the test might end before the
>>>>> unexpected notification arrives.
>>>>
>>>> Hm, you mean providing a proof that extraneous notifications are not
>>>> emitted. I'm not really sure you can create such a proof for the
>>>> existing implementation - even if everything is fine within a certain
>>>> time window it does not imply that in the next n seconds an unexpected
>>>> notification wouldn't be delivered.
>>> Indeed, it is very difficult to make sure no unexpected notification,
>>> but the old version could by chance to get an unexpected because it
>>> waited always certain time.
>>
>> IMO, getting something right by chance is even worse than simply stating
>> that the test won't test for such eventuality.
>>
>>>>
>>>>>
>>>>> Not sure that we should take care of this case.
>>>>
>>>> Probably not in this test. This test just asserts that all the
>>>> expected notifications have been emitted.
>>> No objection. This is a general issue for many other notification tests
>>> too.
>>
>> In general it is impossible to test for extraneous notifications since
>> there are no events cleanly defining the time boundaries for receiving a
>> particular notification. The result is, that even though an extraneous
>> notification hasn't been received in n seconds we can't be certain that
>> one wouldn't arrive in n+m (m is a real number and m > 0) seconds.
>>
>>
>> Could I have a (R)eviewer to take a look at this patch, please?
>>
>> -JB-
>>
>>>
>>> Shanliang
>>>>
>>>> -JB-
>>>>
>>>>>
>>>>> Thanks,
>>>>> Shanliang
>>>>>
>>>>> Jaroslav Bachorik wrote:
>>>>>> Please, review the following test change.
>>>>>>
>>>>>> Issue : https://bugs.openjdk.java.net/browse/JDK-7132590
>>>>>> Webrev: http://cr.openjdk.java.net/~jbachorik/7132590/webrev.00
>>>>>>
>>>>>> Currently, the test waits for an arbitrary time until it gives up on
>>>>>> receiving the notifications. This leads to intermittent failures in
>>>>>> situations when the execution is slower than anticipated (running
>>>>>> against a debug build etc.).
>>>>>>
>>>>>> The solution is to block the test until all the expected notification
>>>>>> had been delivered or the test is timed out by the harness.
>>>>>>
>>>>>> Thanks,
>>>>>>
>>>>>> -JB-
>>>>>
>>>>
>>>
>>
>