Var volatile bug?

[Nathan Fisher]

Hi All,

I found what I *think* to be a bug with the use of the Var.rev static field in the Var constructor and it's increment elsewhere in the Var class. I'm not able to login to Jira but I'll outline the jist of what I think might be the bug below.

There are two similar(-ish) problems with the increment of rev;

1. The increment of rev in the constructor isn't synchronised and a post inc isn't atomic (e.g. not thread-safe).

2. A synchronised instance method does not synchronise a static field. Therefore rev is not thread-safe in bindRoot, swapRoot, unbindRoot, commuteRoot, and alterRoot.

I don't 100% follow the use case of Var.rev. This could be an "as designed" if these methods are guaranteed to only be called in a sequential and serial order. However if it's not as designed I think there's one of a few ways it can be addressed;

1. Use an AtomicInteger.

2. Use explicit synchronised blocks around rev;

// e.g.

synchronised(Var.class) {

 ++rev

}

3. If rev is meant as a synchronisation primitive for the AtomicReference in Namespace.intern, rev could be hoisted to Namespace and replace the AtomicReference with an AtomicStampedReference instead.

I've attached a patch for option 2 which can easily be refactored to option 1 and then to 3. Happy to adjust the patch to any of the above options although 3 is likely to take a little of effort. The patch includes integration tests which can be executed with `mvn verify`.

Can I confirm what code style is being used so no one has to muck about with whitespace on your end?

Kind regards,

Nathan

--

- sent from my mobile

[Nathan Fisher]

* post inc should be pre inc (e.g. ++rev).

[Alex Miller]

You're correct that it's not thread-safe but I don't believe rev is ever used anywhere. It was added in the commit that switched vars to be non-dynamic by default. Maybe this was being used for some debugging in the midst of making that change and should have been removed. Seems like a patch to remove it would be reasonable. 

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.
To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

[Nathan Fisher]

Attached and should be applicable to HEAD of master.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

--

You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

[Alex Miller]

Please file a jira and attach there.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

--

- sent from my mobile

--

You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev+unsubscribe@googlegroups.com.

[Nathan Fisher]

I've been having issues logging into Jira and Confluence. Tried signing up and password resets. Looks like I ran into 2 issues; 1) false assumption that the user store was shared between the two, 2) I use a password safe and generate passwords > 32 chars.

Jira Issue is here;

https://dev.clojure.org/jira/browse/CLJ-2306

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

--

- sent from my mobile

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.

--
You received this message because you are subscribed to the Google Groups "Clojure Dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to clojure-dev...@googlegroups.com.

To post to this group, send email to cloju...@googlegroups.com.
Visit this group at https://groups.google.com/group/clojure-dev.
For more options, visit https://groups.google.com/d/optout.