Hello,
I noticed that LazyTransformer [1] implements mutable state without any
volatile or final modifier to guard its fields (it might not be an isolated
case in the core library e.g. SeqIterator.java).
Isn't such class subject to unsafe publication?
For example, considering only this part of the implementation:
public final class LazyTransformer extends ... {
IStepper stepper;
Object first = null;
LazyTransformer rest = null;
public LazyTransformer(IStepper stepper){
this.stepper = stepper;
}
public static LazyTransformer create(IFn xform, Object coll){
return new LazyTransformer(new Stepper(xform, RT.iter(coll)));
}
...
public Object first(){
if(stepper != null)
seq();
if(rest == null)
return null;
return first;
}
public ISeq next(){
if(stepper != null)
seq();
if(rest == null)
return null;
return rest.seq();
}
..
}
If LazyTransformer/create is called in in one thread and then another
thread calls 'first' or 'next' on the new object then I would say there's a
chance the second thread sees stepper == null. A real use case could be one
thread calling (dedup) [2], which creates lazy sequence with (sequence
<http://crossclj.info/ns/org.clojure/clojure/1.7.0-alpha5/clojure.core.html#_sequence>
(dedupe
<http://crossclj.info/ns/org.clojure/clojure/1.7.0-alpha5/clojure.core.html#_dedupe>)
coll), and then another thread iterates through the sequence, which I
suppose relies on 'first' and 'next'.
To me it looks like the exact same situation as the one discussed on the
Java concurrency-interest here:
http://jsr166-concurrency.10961.n7.nabble.com/Safe-publishing-strategy-tt12126.html
I can't believe I'm the first to be worried by this so maybe I overlooked
something. But if I'm not wrong or if there is any doubt, next to
refactoring the code to use immutable classes with only final fields (my
preferred approach), wouldn't it be safer to declare at least 'stepper' as
volatile to prevent any of risk of nasty concurrency issue?
Thanks,
Sébastien
[1]
https://github.com/clojure/clojure/blob/master/src/jvm/clojure/lang/LazyTransformer.java
[2] http://crossclj.info/fun/clojure.core/dedupe.html
--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to [email protected]
Note that posts from new members are moderated - please be patient with your
first post.
To unsubscribe from this group, send email to
[email protected]
For more options, visit this group at
http://groups.google.com/group/clojure?hl=en
---
You received this message because you are subscribed to the Google Groups
"Clojure" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to [email protected].
For more options, visit https://groups.google.com/d/optout.
