Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-25 Thread 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, bo

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-25 Thread Anthony Petrov
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.invok

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Sergey Bylokhov
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

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Anthony Petrov
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/2

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Sergey Bylokhov
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),

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Anthony Petrov
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. -- best regards, Anthony On 1

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Sergey Bylokhov
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

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Oleg Pekhovskiy
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

Re: [8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Sergey Bylokhov
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 =

[8] Review request for 8028995: Write regression test for JDK-8016356

2013-11-22 Thread Oleg Pekhovskiy
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