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

[Sergey Bylokhov]

On 16.11.16 17:32, Semyon Sadetsky wrote:
>> 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.

Please clarify the statement above.

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

But in some corner cases we change this value, so it cannot be final.

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


-- 
Best regards, Sergey.