<AWT Dev> [8] Review Request: 7090424 TestGlyphVectorLayout failed automately with java.lang.StackOverflowError

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Tue Oct 22 01:28:31 PDT 2013


I have a problem with the review tool. I assume that version .00 with 
removed volatiles and renamed handleexpose event is ok for everybody?

On 10/21/13 4:53 PM, Sergey Bylokhov wrote:
> Hi, Anthony.
> Since cr have some issues with the space. I upload 2 files, where 
> handleExposeEvent was renamed to postPaintEvent():
>
> http://cr.openjdk.java.net/~serb/7090424/webrev%2c01/src/solaris/classes/sun/awt/X11/XWindow.java.sdiff.html 
>
> http://cr.openjdk.java.net/~serb/7090424/webrev%2c01/src/solaris/classes/sun/awt/X11/XContentWindow.java.sdiff.html 
>
>
> On 18.10.2013 15:47, Sergey Bylokhov wrote:
>> Hi, Anthony.
>> On 18.10.2013 15:17, Anthony Petrov wrote:
>>> Hi Sergey,
>>>
>>> In XAWT, we usually use the StateLock to synchronize access to peer 
>>> fields (such as background, label, etc.) I don't think that 
>>> switching to volatile is a good idea since it prevents us from 
>>> performing atomic read/writes to the fields. And this is exactly 
>>> what we need for this fix, actually. In other words, the following 
>>> pattern works perfectly:
>>>
>>> synchronized (lock) {
>>>    if (a != b) {
>>>       a = b;
>>>       // do stuff, or set a flag to do it later w/o the lock
>>>    }
>>> }
>>>
>>> whereas the following doesn't:
>>>
>>> volatile a;
>>> if (a != b) {
>>>    a = b;
>>>    // do stuff
>>> }
>>>
>>> The latter doesn't work because the value of 'a' may change from 
>>> another thread after the if() statement in the first thread is 
>>> executed.
>> If it will be changed that's ok. It is safe in this context since 
>> there is no races. a will be the latest setted value and repaint 
>> action will be done after a was set. Non trivial setters(like 
>> setLabel/setText) are called under the locks in shared code.
>>>
>>> Please note that this is critical for AWT because it is a 
>>> multi-threaded GUI toolkit.
>>>
>>> src/solaris/classes/sun/awt/X11/XListPeer.java
>>>> -                target.paint(g);
>>>> +                handleExposeEvent(target, 0, 0, getWidth(), 
>>>> getHeight());
>>>
>>> (the same applies to XWindow.repaint): can you please rename 
>>> XWindow.handleExposeEvent(Component...) to postPaintEvent() and make 
>>> it final? A good javadoc for this method would also be appreciated, 
>>> because currently seeing the name handle*() I'd think it needs to be 
>>> done under the awtLock().
>> Will do.
>>>
>>> Also, for the corresponding changes in XWindow.repaint(), could you 
>>> please elaborate a bit more? Looking at the code I see that 
>>> XWindow.paint() calls paintPeer(). And in repaint(), you either call 
>>> paint() or paintPeer() depending on the current thread. Why is it 
>>> needed? Can we just call paintPeer() (or paint() for that matter) 
>>> unconditionally since they both seem to result in the same call?
>> paintPeer is used to draw the native part of the peer(text of the 
>> label, border of the button etc).
>> paint is a part of Component.paintAll(). And as a result of it call 
>> it should paint the native part of the peer and it should call 
>> appropriate paint method from the target.
>>> Also, why don't we post an event if reapint() is invoked on the EDT?
>> We do this in osx, but in xawt there are "smart" caches, which are 
>> initialized during the paint of the peer. See 
>> XLabelPeer&cachedFontMetrics for example . Note that this cache is 
>> completely broken.
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>> On 10/16/2013 06:00 PM, Sergey Bylokhov wrote:
>>>> Hello.
>>>> Please review the fix for jdk 8.
>>>> The fix has two parts
>>>>   - Repaint method in the peer now paint the component in place if it
>>>> was called on EDT only.
>>>>   - Most of setters were changed to stop recursion if they were called
>>>> on EDT.
>>>>
>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-7090424
>>>> Webrev can be found at: 
>>>> http://cr.openjdk.java.net/~serb/7090424/webrev.00
>>>>
>>
>>
>
>


-- 
Best regards, Sergey.



More information about the awt-dev mailing list