On 09/27/2013 04:33 PM, Sergey Bylokhov wrote:
On 27.09.2013 16:24, Anthony Petrov wrote:
Hi Alexander,
I'm not sure if this is the right approach because the setter is
overridable. So whenever you update the field, you may actually call
user's code which in most cases is undesirable. Also, you really
shouldn't do this in the context of readObject() because the object
isn't fully initialized yet, so calling overridable methods at this
stage is asking for troubles.
I still suggest to access the field directly, but wrap the accessing
statement with synchronized (getTreeLock()) {}. And I wouldn't modify
the readObject() at all.
I suppose to fix readObject the field can be marked volatile.
In theory, yes. However, a separate bug exists to evaluate
synchronization issues in AWT code, so we'll get back to this issue
(wrt. all fields) later. For now, let's fix the functional part of the
code only.
--
best regards,
Anthony
--
best regards,
Anthony
On 09/27/2013 02:33 PM, Alexander Scherbatiy wrote:
Could you review the updated fix:
http://cr.openjdk.java.net/~alexsch/7092283/webrev.01
- the locationByPlatform property is updated by setter method
- the open and closed tests are passed with the fix except one which
fails without the fix too:
java/awt/Dialog/MakeWindowAlwaysOnTop/MakeWindowAlwaysOnTop.java
Thanks,
Alexandr.
On 9/25/2013 5:37 PM, Anthony Petrov wrote:
Hi Alexander,
The setter and getter for this property access the boolean flag under
the tree lock. I suppose this was the locking design for this field.
So you should reset it under this lock, too. You could also go ahead
and add proper locking to other methods that access it (e.g. show(),
etc.)
Please run Window, Frame, and Dialog automatic regression tests (open
and closed) to ensure no regressions after this change.
--
best regards,
Anthony
On 09/25/2013 04:50 PM, Alexander Scherbatiy wrote:
Hello,
Could you review the fix:
bug: https://bugs.openjdk.java.net/browse/JDK-7092283
webrev: http://cr.openjdk.java.net/~alexsch/7092283/webrev.00
The Window.hide() method does not clear the locationByPlatform
property.
Thanks,
Alexandr.
