Based on several articles about synchronization (Doug Lea) you need
volatile when a thread needs to read the array.
Look also at CopyOnWriteArrayList : any changes is performed under a
synchronized block and any read is performed under volatile
constraints(synchronization with main memory). Not using volatile it is
still thread safe but the usage will not be 100%
correct(http://gee.cs.oswego.edu/dl/cpj/jmm.html, see Volatile)
"Declaring fields as volatile can be useful when you do not need locking
for any other reason, yet values must be accurately accessible across
multiple threads. This may occur when:
* The field need not obey any invariants with respect to others.
* Writes to the field do not depend on its current value.
* No thread ever writes an illegal value with respect to intended
semantics.
* The actions of readers do not depend on values of other
non-volatile fields."
Adding/removing a listener : a synchronized block guarantees atomicity
of changing the array and replacing the old array with a new one.
However reading the array doesn't need syncronization but still need
volatile to "accurately accessible across multiple threads".
public class CopyOnWriteArrayList<E>
implements List<E>, RandomAccess, Cloneable, java.io.Serializable {
private static final long serialVersionUID = 8673264195747942595L;
/**
* The held array. Directly accessed only within synchronized
* methods
*/
private volatile transient E[] array;
Jon Seymour wrote:
Use of synchronized will solve the thread safety issue I was concerned
about. I am not sure that a volatile declaration adds anything in this
case, since the synchronized declaration is a stronger declaration
than volatile [ e.g. synchronized implies volatile from the
perspective of threads that synchronize on the relevant monitor ].
Regards,
jon
On Fri, Aug 1, 2008 at 11:36 PM, Adrian Tarau
<[EMAIL PROTECTED]> wrote:
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
