Merge pull request #202 from tanzoniteblack/let-flow-bound-fn

Use `bound-fn` in `let-flow`
clj-commons · Sep 6, 2021 · f32f4e9 · f32f4e9
2 parents dbdd600 + a21a832
commit f32f4e9
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 6 deletions.
diff --git a/CHANGES.md b/CHANGES.md
@@ -1,8 +1,9 @@
 ### 0.2.0-SNAPSHOT
 
-Contributions by Matthew Davidson
+Contributions by Matthew Davidson, Ryan Smith
 
-* Modernized indentation to match current Clojure styles and fixed misalignments
+* Switch to `bound-fn` in `let-flow` to fix bug where dynamic vars were incorrect for other threads 
+* Modernized indentation to match current Clojure styles and fix misalignments
 
 ### 0.1.9
 

diff --git a/src/manifold/deferred.clj b/src/manifold/deferred.clj
@@ -1330,14 +1330,14 @@
                            [gensym val])
                          [gensym
                           `(~chain-fn (~zip-fn ~@deps)
-                             (fn [[~@(map gensym->var deps)]]
+                             (bound-fn [[~@(map gensym->var deps)]]
                                ~val))])))
                    (range)
                    vars'
                    vals'
                    gensyms)]
            (~chain-fn (onto (~zip-fn ~@body-dep?) executor#)
-             (fn [[~@(map gensym->var body-dep?)]]
+             (bound-fn [[~@(map gensym->var body-dep?)]]
                ~@body)))))))
 
 (defmacro let-flow

diff --git a/test/manifold/deferred_test.clj b/test/manifold/deferred_test.clj
@@ -5,7 +5,8 @@
     [manifold.utils :as utils]
     [clojure.test :refer :all]
     [manifold.test-utils :refer :all]
-    [manifold.deferred :as d]))
+    [manifold.deferred :as d]
+    [manifold.executor :as ex]))
 
 (defmacro future' [& body]
   `(d/future
@@ -72,6 +73,8 @@
               (d/chain #(/ 1 %))
               (d/catch ArithmeticException (constantly :foo))))))
 
+(def ^:dynamic *test-dynamic-var*)
+
 (deftest test-let-flow
 
   (let [flag (atom false)]
@@ -93,7 +96,18 @@
             (d/let-flow [[x] (future' [1])]
               (d/let-flow [[x'] (future' [(inc x)])
                            y (future' true)]
-                (when y x')))))))
+                (when y x'))))))
+
+  (testing "let-flow callbacks happen on different executor retain thread bindings"
+    (let [d (d/deferred (ex/fixed-thread-executor 1))
+          test-internal-fn (fn [] (let [x *test-dynamic-var*]
+                                    (d/future (Thread/sleep 100) (d/success! d x))))]
+      (binding [*test-dynamic-var* "cat"]
+        (test-internal-fn)
+        (is (= ["cat" "cat" "cat"]
+               @(d/let-flow [a d
+                             b (do a *test-dynamic-var*)]
+                  [a b *test-dynamic-var*])))))))
 
 (deftest test-chain-errors
   (let [boom (fn [n] (throw (ex-info "" {:n n})))]
@@ -388,3 +402,4 @@
       (Thread/sleep (rand-int 10))
       (d/success! d 1)
       (is (= 1 @@result)))))
+