You're right, but I've sent a correction after that : adding synchronized to addListener and removeListener. Adding synchronized keyword guarantees atomic modifications of listners array and provides visibility(by committing changes in main memory) for listeners to other threads. Also listeners should be declared volatile to force a sync with main memory.

If I'm mistaken please correct me...

This is(should be) correct :

public class A {

private volatile Listener[] listeners =  new Listener[0] ;

// add a new listener
public synchronized void addListener(Listener listener) {
   listeners = ArrayUtilities.addObject(listeners, listener);
}

// remove a listener
public synchronized void removeListener(Listener listener) {
   listeners = ArrayUtilities.removeObject(listeners, listener);
}

// fire event
public void fireCodeChanged(Event event) {
   Listener[] currentListeners =  this.listeners;
   foreach(Listener listener : listeners) {
       listener.onChange(event);
   }
}




Jon Seymour wrote:
Of course the modification proposed here is not thread safe in its current form.

Jon.

On 7/30/08, Adrian Tarau <[EMAIL PROTECTED]> wrote:
The best way to implement listeners (as you may see in
EventListenerList) is to work with arrays.They are immutable and have
the lowest memory footprint(instead of HashSet/ArrayList or any
concurrent class).

Eventually you can skip copying the listeners array if you manage
everything locally instead of having a class like EventListnerList(need
it - maybe - if you fire events at a really high rate, so you want to
avoid creating an array every time). If you encapsulate array changes in
a class ArrayUtilities you will not add to much code to implement
add/remove/fire listeners.

public class A {

Listener[] listeners =  new Listener[0] ;

// add a new listener
public void addListener(Listener listener) {
    listeners = ArrayUtilities.addObject(listeners, listener);
}

// remove a listener
public void removeListener(Listener listener) {
    listeners = ArrayUtilities.removeObject(listeners, listener);
}

// fire event
public void fireCodeChanged(Event event) {
    Listener[] currentListeners =  this.listeners;
    foreach(Listener listener : listeners) {
        listener.onChange(event);
    }
}

Right now IntrospectorCacheImpl can fail with
ConcurrentModificationExceptions if you don't add synchronized to
addListener and removeListener.

....

Nathan Bubna wrote:
Given the lack of affirmative response, i'm going to take some wild
guesses here...

On Thu, Jul 24, 2008 at 10:34 PM, Nathan Bubna <[EMAIL PROTECTED]> wrote:

1) Does anyone out there actually use custom implementations of the
IntrospectorCache interface?

nope.


2) Does anyone use any IntrospectorCacheListeners?

no.


3) Can anyone give me a use-case for IntrospectorCacheListeners?
(Henning?)

yes, Henning, who is busy with other things.  Henning, if you would
like these back, i'll replace them then.  for now, i'm yanking them.
:)


For those curious about my reasons for asking:  i'm looking at
IntrospectorBase and the classes above and seeing slight risk of
ConcurrentModificationExceptions in the way listeners are handled.
This would be trivially fixed by synchronizing the add/removeListener
methods.  However, i'm hoping to use more fine-grained synchronization
in those classes that would require potentially more hassle than the
risk.  Loads of fun, ya know.  So, if i find out the listeners are
used w/o problems, i won't worry about it.  If i find out they're not
used at all and can't figure out what they're good for, then i would
happily be rid of them. :)

all input is welcome!


---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]




Reply via email to