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
On Monday, March 9, 2015 at 5:40:11 PM UTC-5, Sébastien Bocq wrote:
>
> 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.
