loop and short-circuiting

[prepor]

Hello. Is this code supposed to stop loop execution?

(require '[manifold.deferred :as deferred]
           '[manifold.stream :as stream]
           '[manifold.time :as mt])
  (->
   (deferred/loop []
     (prn "---TICK!")
     (mt/in 200 #(deferred/recur)))
   (deferred/timeout! 1000)
   (deref))

Currently, it loops forever and for me, it looks as broken API. Maybe loop should check result# before each tick?

[ztellman]

Sure, that would be consistent with other parts of the API, I just hadn't ever considered someone would try something like this. PR is welcome, otherwise I'll get to it when I can.

[prepor]

@ztellman I started working on PR. https://github.com/ztellman/manifold/compare/master...prepor:patch-1

For this primitive case it works. Then I put loop into chain and we again have infinity loop:

(->
   (deferred/chain
     (deferred/loop []
       (prn "---TICK!")
       (mt/in 200 #(deferred/recur)))
     (fn [_] :finished))
   (deferred/timeout! 1000)
   (deref))

Then I realized that short-circuiting doesn't work for nested chains too:

(->
   (deferred/chain
     (deferred/chain
       (deferred/future (Thread/sleep 2000))
       (fn [_]
         (prn "----FINISH")
         :finished))
     (fn [_] :finished))
   (deferred/timeout! 1000)
   (deref))
=> Execution error (TimeoutException) at manifold.deferred/timeout!$fn (deferred.clj:1160).
timed out after 1000 milliseconds
(after some time) => "----FINISH"

Is this could be considered as a bug or?

[ztellman]

You'll want the when-not check to be inside the loop, not outside.

[prepor]

@ztellman yes, it makes sense, fixed.

But it doesn't fix issues which I described previously.

[ztellman]

If we had a reified topology of deferreds (like we do streams), we could potentially walk upstream when a given deferred is cancelled and cancel everything that feeds only into that cancelled deferred. Since we don't, there's no easy, general way to address either of the cases you mention. Barring a major reengineering of Manifold, I think we have to be content with the simple cancellation case working.