Hi Semyon,
The fix looks fine to me. Please, add its description to JIRA as well.
Regards,
Anton.
On 15.04.2015 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
