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
