[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-awt-dev
Subject: Re: <AWT Dev> [9] Review request for 7155957: closed/java/awt/MenuBar/MenuBarStress1/MenuBarStress1.
From: Sergey Bylokhov <Sergey.Bylokhov () oracle ! com>
Date: 2015-04-21 9:29:36
Message-ID: 55361880.5040300 () oracle ! com
[Download RAW message or body]
Hi, Semyon.
The fix looks fine to me.
Thanks!
On 21.04.15 11:37, Semyon Sadetsky wrote:
> Sergey,
>
> done.
> http://cr.openjdk.java.net/~ssadetsky/7155957/webrev.01/
>
> --Semyon
>
> On 4/16/2015 6:41 PM, Sergey Bylokhov wrote:
>> Hi, Semyon.
>> Please mark all related fields in WObjectPeer as volatile( pData/
>> destroyed/etc). Probably MenuComponent.font field also?
>> That could be a reason for similar random issues.
>>
>> On 15.04.15 17:38, Semyon Sadetsky wrote:
>>> Hello,
>>>
>>> Please review fix for JDK9.
>>> webrev: http://cr.openjdk.java.net/~ssadetsky/7155957/webrev.00/
>>> bug: https://bugs.openjdk.java.net/browse/JDK-7155957
>>>
>>> *THE ROOT CAUSE
>>> A number of concurrency bugs in AWT Toolkit.
>>> When menus are modified concurrently there is a big chance that the
>>> internal Windows menu events are handled at the same time on the
>>> AWT-Windows thread which is not synchronized. If the internal event
>>> processing on AWT-Windows calls Java side methods the JVM can die
>>> silently.
>>>
>>> *SOLUTIONS
>>> A number of fixes in various Java and C++ classes are introduced to
>>> eliminate (with finite probability) concurrency problems :
>>>
>>> 1. java.awt.Menu.remove(int)
>>>
>>> The peer.delItem(index) is called after mi.removeNotify(). That
>>> means that dispose event will be sent earlier than the menu remove
>>> WinAPI call. This causes Access Violation exception because Windows
>>> events may come with deallocated references.
>>> The solution is to call them in the right order.
>>>
>>> 2. java.awt.MenuBar.remove(int)
>>> Same error as in 1 for menu bar.
>>>
>>>
>>> 3. java.awt.MenuComponent.serFont(Font)
>>> This method should hold tree lock while running otherwise its
>>> concurrent execution causes Access Violation in number of places and
>>> JVM is crashed.
>>>
>>> 4. awt_Menu.cpp
>>> AwtMenu::GetItem(jobject target, jint index), AwtMenu::DrawItems(),
>>> AwtMenu::MeasureItems(
>>>
>>> Calling java.awt.Menu.getItem() during internal windows event
>>> processing can throw ArrayIndexOutOfBoundException because number of
>>> menu items could be changed concurrently and the index is not in
>>> range. This causes a hidden exception which is only seen in debug
>>> mode as an Assertion error.
>>>
>>> Another issue here is request to GetPeerForTarget() for the menu
>>> item peer which can be concurrently deleted on the Java side as
>>> result of Menu.delNotify() execution. It causes NPE peer which is
>>> also hidden and only reveals itself as an Assertion error in debug
>>> mode.
>>>
>>> The solution for all is to abort and return windows event callback
>>> if the menu structure was changed concurrently.
>>>
>>> 5. awt_MenuBar.cpp AwtMenuBar::GetItem()
>>>
>>> Same solution as 4 for menu bar in similar situations.
>>>
>>> 6. awt_MenuItem.cpp AwtMenuItem::Dispose()
>>>
>>> The "destroyed" filed should be set for the peer before pData is set
>>> to NULL otherwise "NPE null pData" can be thrown in various
>>> concurrent situations.
>>>
>>> 7. awt_new.cpp safe_ExceptionOccurred()
>>>
>>> This routine is called evrywere in the code to check exceptions. It
>>> only stops execution and prints to console if OOE happened, but
>>> other exceptions are re-thrown silently and execution continues
>>> without any warnings. If the call is initiated by an internal
>>> Windows event callback the exceptions are hidden in the release mode
>>> and shown in the debug mode as the Assertion Error message box, but
>>> in the last case without any useful information because
>>> GetLastError() is always 0 in such situations.
>>> I have added env->ExceptionDescribe() to print exception stack trace
>>> on the console for all debug and release modes. This should help to
>>> detect internal toolkit issues during testing by JCK and jtreg.
>>> Later before the JDK9 release we can leave it for debug mode only.
>>>
>>> *A KNOWN PROBLEM DID NOT FIXED
>>> When font is assigned to a menu item concurrently there is a big
>>> chance that menu item size will be calculated with one font while
>>> drawing of the item will be performed with another font. In such
>>> situation label of the menu item does not fit its size or vice versa.
>>> This is due to nature how the Windows OS handles owner-drawn menus.
>>>
>>> --Semyon
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>
>>
>> --
>> Best regards, Sergey.
>
--
Best regards, Sergey.
[Attachment #3 (text/html)]
<html>
<head>
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<div class="moz-cite-prefix">Hi, Semyon.<br>
The fix looks fine to me.<br>
Thanks!<br>
<br>
On 21.04.15 11:37, Semyon Sadetsky wrote:<br>
</div>
<blockquote cite="mid:55360C3E.6020602@oracle.com" type="cite">
<meta content="text/html; charset=utf-8" http-equiv="Content-Type">
Sergey,<br>
<br>
done. <br>
<a moz-do-not-send="true" class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Essadetsky/7155957/webrev.01/">http://cr.openjdk.java.net/~ssadetsky/7155957/webrev.01/</a><br>
<br>
--Semyon<br>
<br>
<div class="moz-cite-prefix">On 4/16/2015 6:41 PM, Sergey Bylokhov
wrote:<br>
</div>
<blockquote cite="mid:552FD82D.3020208@oracle.com" type="cite">
<meta content="text/html; charset=utf-8"
http-equiv="Content-Type">
<div class="moz-cite-prefix">Hi, Semyon.<br>
Please mark all related fields in
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
WObjectPeer as volatile(
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
pData/
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
destroyed/etc). Probably
<meta http-equiv="content-type" content="text/html;
charset=utf-8">
MenuComponent.font field also?<br>
That could be a reason for similar random issues.<br>
<br>
On 15.04.15 17:38, Semyon Sadetsky wrote:<br>
</div>
<blockquote cite="mid:552E77E9.3060906@oracle.com" type="cite">Hello,
<br>
<br>
Please review fix for JDK9. <br>
webrev: <a moz-do-not-send="true"
class="moz-txt-link-freetext"
href="http://cr.openjdk.java.net/%7Essadetsky/7155957/webrev.00/">http://cr.openjdk.java.net/~ssadetsky/7155957/webrev.00/</a>
<br>
bug: <a moz-do-not-send="true" class="moz-txt-link-freetext"
href="https://bugs.openjdk.java.net/browse/JDK-7155957">https://bugs.openjdk.java.net/browse/JDK-7155957</a>
<br>
<br>
*THE ROOT CAUSE <br>
A number of concurrency bugs in AWT Toolkit. <br>
When menus are modified concurrently there is a big chance
that the internal Windows menu events are handled at the same
time on the AWT-Windows thread which is not synchronized. If
the internal event processing on AWT-Windows calls Java side
methods the JVM can die silently. <br>
<br>
*SOLUTIONS <br>
A number of fixes in various Java and C++ classes are
introduced to eliminate (with finite probability) concurrency
problems : <br>
<br>
1. java.awt.Menu.remove(int) <br>
<br>
The peer.delItem(index) is called after mi.removeNotify().
That means that dispose event will be sent earlier than the
menu remove WinAPI call. This causes Access Violation
exception because Windows events may come with deallocated
references. <br>
The solution is to call them in the right order. <br>
<br>
2. java.awt.MenuBar.remove(int) <br>
Same error as in 1 for menu bar. <br>
<br>
<br>
3. java.awt.MenuComponent.serFont(Font) <br>
This method should hold tree lock while running otherwise its
concurrent execution causes Access Violation in number of
places and JVM is crashed. <br>
<br>
4. awt_Menu.cpp <br>
AwtMenu::GetItem(jobject target, jint index),
AwtMenu::DrawItems(), AwtMenu::MeasureItems( <br>
<br>
Calling java.awt.Menu.getItem() during internal windows event
processing can throw ArrayIndexOutOfBoundException because
number of menu items could be changed concurrently and the
index is not in range. This causes a hidden exception which is
only seen in debug mode as an Assertion error. <br>
<br>
Another issue here is request to GetPeerForTarget() for the
menu item peer which can be concurrently deleted on the Java
side as result of Menu.delNotify() execution. It causes NPE
peer which is also hidden and only reveals itself as an
Assertion error in debug mode. <br>
<br>
The solution for all is to abort and return windows event
callback if the menu structure was changed concurrently. <br>
<br>
5. awt_MenuBar.cpp AwtMenuBar::GetItem() <br>
<br>
Same solution as 4 for menu bar in similar situations. <br>
<br>
6. awt_MenuItem.cpp AwtMenuItem::Dispose() <br>
<br>
The "destroyed" filed should be set for the peer before pData
is set to NULL otherwise "NPE null pData" can be thrown in
various concurrent situations. <br>
<br>
7. awt_new.cpp safe_ExceptionOccurred() <br>
<br>
This routine is called evrywere in the code to check
exceptions. It only stops execution and prints to console if
OOE happened, but other exceptions are re-thrown silently and
execution continues without any warnings. If the call is
initiated by an internal Windows event callback the exceptions
are hidden in the release mode and shown in the debug mode as
the Assertion Error message box, but in the last case without
any useful information because GetLastError() is always 0 in
such situations. <br>
I have added env->ExceptionDescribe() to print exception
stack trace on the console for all debug and release modes.
This should help to detect internal toolkit issues during
testing by JCK and jtreg. Later before the JDK9 release we can
leave it for debug mode only. <br>
<br>
*A KNOWN PROBLEM DID NOT FIXED <br>
When font is assigned to a menu item concurrently there is a
big chance that menu item size will be calculated with one
font while drawing of the item will be performed with another
font. In such situation label of the menu item does not fit
its size or vice versa. <br>
This is due to nature how the Windows OS handles owner-drawn
menus. <br>
<br>
--Semyon <br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
<br>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Best regards, Sergey. </pre>
</blockquote>
<br>
</blockquote>
<br>
<br>
<pre class="moz-signature" cols="72">--
Best regards, Sergey. </pre>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic