Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Possible issue with alt #141

Open
stoyle opened this issue Sep 13, 2017 · 4 comments
Open

Possible issue with alt #141

stoyle opened this issue Sep 13, 2017 · 4 comments

Comments

@stoyle
Copy link

stoyle commented Sep 13, 2017

I am making a process which will run periodically, but it should also be possible to kill it. This is the code:

(defn foo [kill-d]
  (d/loop []
    (prn "Setting up with timeout" 1000)
    (d/let-flow [res (d/alt kill-d (d/timeout! (d/deferred) 1000 :ok))]
      (prn "Got a result" res)
      (if (= res :ok)
        (d/recur)
        (prn "Got a kill message for token, not recurring")))))

When running the alt does not trigger on the timeout.

(def kill (d/deferred))
=> #'user/kill
(foo kill)
"Setting up with timeout" 1000
=> << … >>
;; Witing a bit...

(d/success! kill 1)
"Got a result" 1
"Got a kill message for token, not recurring"

Modifying the code, removing the kill-d from the alt, now it works, except there is no way of killing it;

(defn foo [kill-d]
  (d/loop []
    (prn "Setting up with timeout" 1000)
    (d/let-flow [res (d/alt (d/timeout! (d/deferred) 1000 :ok))]
      (prn "Got a result" res)
      (if (= res :ok)
        (d/recur)
        (prn "Got a kill message for token, not recurring")))))

(def kill (d/deferred)) ; Not really necessary...
=> #'user/kill
(foo kill)
"Setting up with timeout" 1000
=> << … >>
"Got a result" :ok
"Setting up with timeout" 1000
"Got a result" :ok
"Setting up with timeout" 1000
"Got a result" :ok
"Setting up with timeout" 1000
"Got a result" :ok
"Setting up with timeout" 1000
"Got a result" :ok
"Setting up with timeout" 1000
"Got a result" :ok
"Setting up with timeout" 1000

I tried to simplify and it seems maybe timeout isn't the problem. Here alt only has to wait for two deferreds, and only seem to react on the last one:

(defn foo [d1 d2]
  (d/loop []
    (d/let-flow [res (d/alt d1 d2)]
      (prn "Got a result" res)
      (if (= res :ok)
        (d/recur)
        (prn "Got a kill message for token, not recurring")))))

=> #'user/foo
(def a (d/deferred))
=> #'user/a
(def b (d/deferred))
=> #'user/b
(foo a b)
=> << … >>
(d/success! a 1)
=> true
(d/success! b 1)
"Got a result" 1
"Got a kill message for token, not recurring"
=> true
@dm3
Copy link
Contributor

dm3 commented Sep 13, 2017

This seems to be a problem with loop/let-flow macro interaction and alt. If you convert the example to the explicit CPS form, the alt works fine. You can use this as an immediate fix for the problem:

(defn foo-chain [kill-d]
  (d/loop []
    (prn "Setting up with timeout" 1000)
    (-> (d/alt kill-d (d/timeout! (d/deferred) 1000 :ok))
        (d/chain
          (fn [res]
            (prn "Got a result" res)
            (if (= res :ok)
              (d/recur)
              (prn "Got a kill message for token, not recurring")))))))

I'll look into this closer a bit later.

@dm3
Copy link
Contributor

dm3 commented Sep 13, 2017

Here's a testcase:

(deftest alt-letflow
  (let [a (d/deferred), b (d/deferred), result (atom nil)]

    #_(d/let-flow [x (d/alt a b)]
      (reset! result x))

    (-> (d/alt a b)
        (d/chain #(reset! result %)))

    (d/success! a 1)
    (is (= 1 @result))

    (d/success! b 2)
    (is (= 1 @result))))

@dm3
Copy link
Contributor

dm3 commented Sep 13, 2017

So, this happens because let-flow transforms not only its own binding, but also picks up any locals in its lexical scope. In the macroexpansion of the test-case above, a and b get d/zipped together which defeats the purpose of alt.

I don't see a way to fix this without changing let-flow in complicated ways. Pinging @ztellman for an expert opinion :)

@stoyle
Copy link
Author

stoyle commented Sep 15, 2017

In core.async it is idiomatic to use let with alt! and alts!. And I thought manifolds alt was a similar concept. I generally also think let-flow reads better than chain.

If it cannot work, for some reasons, I think it should be properly documented. Spent a bit of time trying to figure out why it wasn't working, and it was not at all obvious where the error was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants