Tim Ellison wrote:
Kevin Zhou wrote:
Tim Ellison wrote:
Hi Kevin,

Kevin Zhou wrote:
I think this defect may be a thread-safe problem.
I read HARMONY's PropertyChangeSupport code and found that:
It employs ArrayList to store the property change listeners which is not
thread-safe.
This defect occurs in a private doFirePropertyChange method.
This method may asynchronously use the list of
PropertyChangeListener, which
contains a null value when modification (add or remove) of the list
has not
finished.
Yes, I put in some trace code and there is a null stored in the
globalListeners list which causes the NPE.

However, I don't see where that null could have been stored.  Can you
suggest what part of the code has stored the null in there?  Every add
to the list first checks for null.  I'll take a look and see if I can
catch it in the test.

I won't apply your 'check-for-null' patch since I think that just masks
the underlying problem.

Regards,
Tim

Hi Tim,
This patch doesn't only check-for-null.
It also changes the private doFirePropertyChange method to synchronized
method. This may resolve the asynchronous operation on globalListeners
list.

Yes, I saw that. How will that help?

I will try to find what part of the code has stored the null.
Due to license cares, I think it difficult to find the setting-null code
which may be inside SwingSet2.jar.

But the globalListeners is a private variable, so the only way it can be
given a null element is if the code successfully calls a method that
adds a null, or we let the private List escape.  I don't see either of
those in the Beans code, do you?  Clearly I'm missing something for this
to happen.

Regards,
Tim

Hi Tim,

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

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?
Thanks :)

Reply via email to