On 16.11.16 19:21, Semyon Sadetsky wrote:
I can give you an example:
CheckboxMenuItem.java
243 public synchronized void addItemListener(ItemListener l) {
244 if (l == null) {
245 return;
246 }
247 itemListener = AWTEventMulticaster.add(itemListener, l);
248 newEventsOnly = true;
249 }
we are enabling event by calling the method above on thread A. The
method is synchronized.
at the same time on thread B dispatchEventImpl(AWTEvent e) is called
which is not synchronized at all.
If thread A is paused by the scheduler at line 248 then thread B may be
executed and the event will be skipped in dispatchEventImpl(AWTEvent e)
because of newEventsOnly=false still.
By making newEventsOnly and itemListener fields volatile you only
guarantee their visibility for thread B but this doesn't make
newEventsOnly change atomic along with adding an item listener.
The example above produce the same result as if the thread B will call
dispatchEventImpl() early than addItemListener() was called by thread A.
And this is correct behavior(the new events will be proceeded only when
we set newEventsOnly to true).
addItemListener is synchronized, because we need to synchronize access
to the list of listeners "itemListener" when we add/remove listener.
Same for enableEvents(long eventsToEnable) method.
But in some corner cases we change this value, so it cannot be final.
What is that corner case? The comment clearly states that it is never
changed.
We have a setter and we call it in applets, trayicon and in X11.
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.
Also it seems Menu#isHelpMenu field is never used except for toString()
and may be removed.
Why it can be removed since it is used in the toString()?
--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.
