Concurrent with-redefs do not unwind properly, leaving a var permanently changed

Question

On 1.4 and latest master:

user> (defn ten (link: ) 10)

'user/ten

user> (doall (pmap #(with-redefs (link: ten (fn [) %)] (ten)) (range 20 100)))
(20 21 22 23 24 25 34 27 28 29 30 31 32 33 34 35 36 37 38 39 39 35 42 43 44 45 48 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 79 87 88 89 90 91 92 93 94 95 97 92 98 99)
user> (ten)
79

Not sure if this is a bug per se, but the doc doesn't mention lack of concurrency safety and my expectation was that the original value would always be restored after any (arbitrarily interleaved) sequence of with-redefs calls.

Answer 1

Comment made by: timmc

The with-redefs doc (v1.4.0) says "These temporary changes will be visible in all threads." That sounds non-thread-safe to me.

In general, changes to var root bindings are not thread safe.

Answer 2

Comment made by: bendlas

As I understand it, with-redefs is mainly used in test suites to mock out vars. It was introduced when vars became static by default and a lot of testsuites had been using binding for mocking. Maybe the docstring should be amended with something along the lines of: When using this you have to ensure that only a single thread is interacting with redef'd vars.

Answer 3

Comment made by: stu

Behavior find as is, doc string change would be fine.

Answer 4

Comment made by: jafingerhut

Patch clj-1104-doc-unsafety-of-concurrent-with-redefs-v1.txt dated Nov 25 2012 updates doc string of with-redefs to make it clear that concurrent use is unsafe.

Answer 5

Reference: https://clojure.atlassian.net/browse/CLJ-1104 (reported by jawolfe)