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 clojure@googlegroups.com
Note that posts from new members are moderated - please be patient with your 
first post.
To unsubscribe from this group, send email to
clojure+unsubscr...@googlegroups.com
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 clojure+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to