Risk of unsafe publishing of LazyTransformer?

[Sébastien Bocq]

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

[Alex Miller]

Hi Sébastien,

Most publication in LazyTransformer is handled via the synchronized seq() method. However, LazyTransformer is being replaced as part of http://dev.clojure.org/jira/browse/CLJ-1669 so probably moot.

Alex

[Sébastien Bocq]

No worries then. Thanks!

--
You received this message because you are subscribed to the Google
Groups "Clojure" group.
To post to this group, send email to clo...@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+u...@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 a topic in the Google Groups "Clojure" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/clojure/rJiR_Dz9hX8/unsubscribe.
To unsubscribe from this group and all its topics, send an email to clojure+u...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

--

Sébastien