Skip to content

Commit febc716

Browse files
committed
Fix dropped errors in manifold.time/in
The result deferred `d` may end up in an error state when `(f)` throws an exception or returns an error deferred. Now since the cancellation callback was only attached via `d/chain` for side-effect (i.e. the resulting deferred is not returned), the dropped error detection would be triggered in this case. The fix is simply to also attach an error handler with `d/on-realized`. Furthermore, putting the result deferred in an error state would not have cancelled the timer because there was no error handler attached to it. This is now also fixed and the tests are extended to cover the cancellation API, too. The latter uncovered an oversight in the mock clock implementation where the `in` method didn't return a cancellation function, resulting in an NPE. This is now also fixed.
1 parent 72522ea commit febc716

File tree

2 files changed

+29
-5
lines changed

2 files changed

+29
-5
lines changed

src/manifold/time.clj

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@
125125
;;;
126126

127127
(in-ns 'manifold.deferred)
128-
(clojure.core/declare success! error! deferred realized? chain connect)
128+
(clojure.core/declare success! error! deferred realized? chain connect on-realized)
129129
(in-ns 'manifold.time)
130130

131131
;;;
@@ -191,8 +191,14 @@
191191
(reify
192192
IClock
193193
(in [this interval-millis f]
194-
(swap! events update-in [(p/+ ^double @now interval-millis)] #(conj (or % []) f))
195-
(advance this 0))
194+
(let [t (p/+ ^double @now interval-millis)
195+
cancel-fn (fn []
196+
(swap! events #(cond-> %
197+
(contains? % t)
198+
(update t disj f))))]
199+
(swap! events update t (fnil conj #{}) f)
200+
(advance this 0)
201+
cancel-fn))
196202
(every [this delay-millis period-millis f]
197203
(assert (p/< 0.0 period-millis))
198204
(let [period (atom period-millis)
@@ -263,7 +269,9 @@
263269
(catch Throwable e
264270
(manifold.deferred/error! d e)))))
265271
cancel-fn (.in *clock* interval f)]
266-
(manifold.deferred/chain d (fn [_] (cancel-fn)))
272+
(manifold.deferred/on-realized d
273+
(fn [_] (cancel-fn))
274+
(fn [_] (cancel-fn)))
267275
d))
268276

269277
(defn every

test/manifold/time_test.clj

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,23 @@
2424
(testing "delayed function returns failed deferred"
2525
(let [d (d/deferred)]
2626
(d/error! d (Exception. "BOOM"))
27-
(is (thrown? Exception @(t/in 1 (fn [] d)))))))
27+
(is (thrown? Exception @(t/in 1 (fn [] d))))))
28+
29+
(testing "cancelling by completing result deferred"
30+
(let [c (t/mock-clock 0)]
31+
(t/with-clock c
32+
(testing "with success"
33+
(let [n (atom 0)
34+
d (t/in 1 #(swap! n inc))]
35+
(d/success! d :cancel)
36+
(t/advance c 1)
37+
(is (= 0 @n))))
38+
(testing "with error"
39+
(let [n (atom 0)
40+
d (t/in 1 #(swap! n inc))]
41+
(d/error! d (Exception. "cancel"))
42+
(t/advance c 1)
43+
(is (= 0 @n))))))))
2844

2945
(deftest test-every
3046
(let [n (atom 0)

0 commit comments

Comments
 (0)