<AWT Dev> [9] Review request for 7155957: closed/java/awt/MenuBar/MenuBarStress1/MenuBarStress1.java hangs on win 64 bit with jdk8

[Semyon Sadetsky]

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