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

Anthony Petrov anthony.petrov at oracle.com
Fri Oct 18 04:17:43 PDT 2013


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.

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().

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? Also, 
why don't we post an event if reapint() is invoked on the EDT?

--
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
>


More information about the awt-dev mailing list