leonoel / missionary

A functional effect and streaming system for Clojure/Script
Eclipse Public License 2.0
651 stars 26 forks source link

`ap` bug #86

Open leonoel opened 1 year ago

leonoel commented 1 year ago
(def input (m/observe (fn [!] (def i! !) #(do))))

(def ps
  ((m/ap
     (m/?< input)
     (try (loop []
            (m/amb (m/? m/never)
              (recur)))
          (catch Cancelled _
            (m/amb))))
   #(prn :ready) #(prn :done)))
(i! :a)
(i! :b)                                                   ;; infinite loop
lotuc commented 1 month ago

The bug trace down to the infinite loop when itering over Processor (when there is only one Processor, it loops infinitely).

https://github.com/leonoel/missionary/blob/45fcaacd85cc8e6eed46b6c9e1b49f4fec181570/java/missionary/impl/Ambiguous.java#L175

And also, there are two more while loops like that one. I may need to dig a little more to check if they're necessary to be checked. (itering branch in walk and itering choice in cancel). But I add the similar check anyway here)

I can submit a PR if that's acceptable.

leonoel commented 1 month ago

I created branch ap-infinite-loops and pushed this failing test. You can submit PRs on this branch.

@lotuc thank you for tracing this issue. Your fix looks correct to me, I think it can be improved by moving the tail check before the do-while, to avoid re-traversing the entire circular linked list.

The other loops L166 L191 likely have the same issue, but I'd like to prove it with a test before applying the same treatment.