Hi,
I've just hit this very same defect and can recreate it 2 out of 3 runs.
I've tested the patch [1] and this fixes the SwingSet case. It looks
like it is indeed the lack of synchronization on ArrayList that causes
the problem. Looking at the ArrayList code there are lines where we do
things like:
array[firstIndex++] = null;
so it is quite possible that we could hit a timing hole between
nullifying the entry and updating the index giving us the
NullPointerException in this JIRA. If noone has any objections Ill apply
the patch soon.
Regards,
Oliver
[1]
Index: PropertyChangeSupport.java
===================================================================
--- PropertyChangeSupport.java (revision 774748)
+++ PropertyChangeSupport.java (working copy)
@@ -256,8 +256,11 @@
}
// Collect up the global listeners
- PropertyChangeListener[] gListeners = globalListeners
- .toArray(new PropertyChangeListener[0]);
+ PropertyChangeListener[] gListeners;
+ synchronized(this) {
+ gListeners = globalListeners.toArray(new
PropertyChangeListener[0]);
+ }
+
// Fire the events for global listeners
for (int i = 0; i < gListeners.length; i++) {
gListeners[i].propertyChange(event);
Tim Ellison wrote:
Kevin Zhou wrote:
The globalListeners can only be modified by synchronized
"addPropertyChangeListener" and "removePropertyChangeListener".
In addition, these listeners may also be serialized or deserialized by
some applications via "writeObject" and "readObject" methods.
But all of the above methods has excluded the null value from
globalListeners.
Yep.
In addition, since this defect can not be reproduced at my site. Thus I
don't think it can be given a null in Beans code.
If any code can add a null value, this should also occurs on my site.
I'll try to recreate it too. While it was reliably reproducible for me,
I've since rebuilt and can't reproduce it now ;-/
Assume one scenario as follows:
If one thread calls doFirePropertyChange method, successfully acquires
an array of all the global listeners and stops at line263; another
thread calls removePropertyChangeListener; the 1st thread start to call
propertyChange method, but one of listeners in the acquired array is
null, then it throws NPE. (This may also happen when calling
addPropertyChangeListener method.)
Do you think this could happen on our code?
I don't think this can happen.
When the globalListeners list is copied into the gListeners array (lines
259/260) then any removals from globalListeners will not affect gListeners.
In theory, since ArrayList is not synchronized, we could be unlucky and
get a remove() during the toArray() that causes problems.
I wouldn't want to synchronize the whole doFirePropertyChange() method,
since the propertyChange() callback may take some indeterminate time to
execute and we don't want to lock the list for long. We could
synchronize(this) for the .toArray() call.
Regards,
Tim
--
Oliver Deakin
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
