<AWT Dev> [9] Review Request: 8165769 Hang in the help menu item

[Semyon Sadetsky]

On 16.11.2016 16:05, Sergey Bylokhov wrote:

> Hello.
>
> Please review the fix for jdk9.
>
> There are a few issues which can cause a hang in some of our menu 
> stress tests:
>  - Lack in synchronization in the Menu, we tried to synchronized the 
> write of fields, but skip synchronization when we read the fields(in 
> getters for example). In the fix most of the fields were marked as 
> final(if possible) or volatile.
Since the synchronized blocks cannot be simply omitted the adding 
volatile keyword to the fields doesn't improve multithreading capability 
of menu components and the most of the proposed changes practically useless.

Changing package private accesses to private in general improves 
encapsulation and may be approved. But multithreading issues should be 
fixed differently.

Actually the MenuComponent appContext field is never changed according 
to its spec:
/**
      * The {@code AppContext} of the {@code MenuComponent}.
      * This is set in the constructor and never changes.
      */
     transient AppContext appContext;

I suggest to declare it final and also this will be enough to make it 
thread safe.

Note that Component appContext field can be changed and this 
multithreading issue should be resolved. Since the filed is accessed 
only using the component accessor it should be enough to synchronize the 
corresponding getter and setter.

--Semyon
>  - When the submenu is removed from Menu/MenuBar we do not reset its 
> parent field if the Menu/MenuBar have peer==null. So if later we tried 
> to call MenuBar.setHelpMenu(submenu) we skip this submenu because we 
> think it was added already.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8165769
> Webrev can be found at: 
> http://cr.openjdk.java.net/~serb/8165769/webrev.00
>