<AWT Dev> [8] Review request for 8028995: Write regression test for JDK-8016356

[Oleg Pekhovskiy]

Thanks you guys for such a cognitive review!

Oleg.

On 25.11.2013 13:42, Anthony Petrov wrote:
> Thanks for the clarification, Sergey. This sounds reasonable. I'm OK
> with the fix then.
>
> --
> best regards,
> Anthony
>
> On 11/22/2013 08:47 PM, Sergey Bylokhov wrote:
>> It is not just block of the thread, both threads are synchronized via
>> wait/notifyAll() on AWTInvocationLock {} inside
>> EventQueue.invokeAndWait/.
>>
>> http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html
>> ....
>> The |wait| methods of class |Object| (§17.2.1
>> <http://docs.oracle.com/javase/specs/jls/se7/html/jls-17.html#jls-17.2.1>)
>>
>> have lock and unlock actions associated with them; their
>> /happens-before/ relationships are defined by these associated actions.
>> ....
>>
>>   *
>>
>>     An unlock on a monitor /happens-before/ every subsequent lock on
>>     that monitor.
>>
>>   *
>>
>>     A write to a |volatile| field (§8.3.1.4
>>
>> <http://docs.oracle.com/javase/specs/jls/se7/html/jls-8.html#jls-8.3.1.4>)
>>
>>     /happens-before/ every subsequent read of that field.
>>
>> ....
>>
>> Synchronization is required when we block main thread via Thread.sleep()
>> and use ActionListeners() on EDT;
>>
>>
>> On 22.11.2013 20:20, Anthony Petrov wrote:
>>> All I know is as long as you access a variable from multiple threads,
>>> you must use synchronization (locks or volatile). I've never heard
>>> that simply blocking a thread is equivalent to using synchronization
>>> primitives. Are there any documents that specify that?
>>>
>>> --
>>> best regards,
>>> Anthony
>>>
>>> On 11/22/2013 07:55 PM, Sergey Bylokhov wrote:
>>>> On 22.11.2013 18:51, Anthony Petrov wrote:
>>>>> Hi Oleg,
>>>>>
>>>>> The frLoc and frSize (and maybe other variables) should be declared
>>>>> volatile. Generally, if you access a variable from different thread,
>>>>> you have to synchronize the access using either the volatile modifier
>>>>> (the easiest way, esp. for tests), or a lock.
>>>> I always thought that in such cases two threads already synchronized,
>>>> because invokeAndWait is used, which block one thread and wait another.
>>>>>
>>>>> --
>>>>> best regards,
>>>>> Anthony
>>>>>
>>>>> On 11/22/2013 06:35 PM, Sergey Bylokhov wrote:
>>>>>> Thanks!
>>>>>> The fix looks good.
>>>>>>
>>>>>> On 22.11.2013 18:19, Oleg Pekhovskiy wrote:
>>>>>>> Hi Sergey,
>>>>>>>
>>>>>>> thanks you for the review,
>>>>>>> please take a look at the next version of fix:
>>>>>>> http://cr.openjdk.java.net/~bagiras/8028995.2/
>>>>>>>
>>>>>>> Changes:
>>>>>>> 1. OSInfo used.
>>>>>>> 2. Location and Dimensions of Frame are retrieved on EDT.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Oleg
>>>>>>>
>>>>>>> On 22.11.2013 17:16, Sergey Bylokhov wrote:
>>>>>>>> Hi, Oleg.
>>>>>>>> Usually we check the type of OS via sun.awt.OSInfo
>>>>>>>> Note that all these things should be on EDT as well:
>>>>>>>>
>>>>>>>>    // Retrieving the color of window expanded area
>>>>>>>>    86                 p = frame.getLocationOnScreen();
>>>>>>>>    87                 d = frame.getSize();
>>>>>>>>    88                 Insets insets = frame.getInsets();
>>>>>>>>
>>>>>>>>    91
>>>>>>>>    92                 frame.dispose();
>>>>>>>>
>>>>>>>>
>>>>>>>> On 22.11.2013 16:37, Oleg Pekhovskiy wrote:
>>>>>>>>> Hi all,
>>>>>>>>>
>>>>>>>>> please review the fix
>>>>>>>>> http://cr.openjdk.java.net/~bagiras/8028995.1/
>>>>>>>>> for
>>>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8028995
>>>>>>>>>
>>>>>>>>> It's just a regression test for JDK-8016356.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Oleg
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>
>> --
>> Best regards, Sergey.
>>