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

ev/select still revives fibers.... #1409

Closed
amano-kenji opened this issue Feb 20, 2024 · 3 comments
Closed

ev/select still revives fibers.... #1409

amano-kenji opened this issue Feb 20, 2024 · 3 comments
Labels
bug This is not expected behavior, and needs fixing

Comments

@amano-kenji
Copy link
Contributor

amano-kenji commented Feb 20, 2024

In production code, when ev/select is called again, It even runs defer forms after they are run once...... They shouldn't be run again.... Because of this issue, I eliminated channel/from-each and ev/select in my production code.

This is my minimal example.

(import spork/stream)
(import spork/sh)
(import spork/channel)

(def chan (ev/chan))

(defn revive-fibers
  []
  (def /dev/null (sh/devnull))
  (defer (do
           (:close /dev/null)
           (eprint "/dev/null closed"))
    (def proc (os/spawn ["sleep" "10000000000"]
                        :p {:out :pipe :err /dev/null}))
    (defer (do
             (:close proc)
             (eprint "proc closed"))
      (def lines (channel/from-each (stream/lines (proc :out))))
      (ev/spawn
        (ev/sleep 2)
        (:kill proc false :term))
      (match (ev/select chan lines)
        [:close c]
        (eprint "lines closed naturally")))))

(def supervisor (ev/chan))
(ev/spawn
  (forever
    (let [[status fiber] (ev/take supervisor)]
      (eprintf "status: %n, fiber: %n, fiber/last-value: %n" status fiber
               (fiber/last-value fiber)))))
(ev/go revive-fibers nil supervisor)

Output is

status: :ok, fiber: <fiber 0x55CFED05D6C0>, fiber/last-value: <core/process 0x55CFED0595F0>
status: :ok, fiber: <fiber 0x55CFED05D010>, fiber/last-value: nil
lines closed naturally
status: :ok, fiber: <fiber 0x55CFED05D340>, fiber/last-value: nil
status: :error, fiber: <fiber 0x55CFED05D340>, fiber/last-value: nil
proc closed
/dev/null closed
status: :ok, fiber: <fiber 0x55CFED05CB10>, fiber/last-value: nil

I noticed that the iterable task of channel/from-each was not really cancelled because ev/select didn't return a value in channel/from-each.

@amano-kenji amano-kenji changed the title ev/select still tries to revive dead fibers.... ev/select still revives dead fibers.... Feb 20, 2024
@amano-kenji amano-kenji changed the title ev/select still revives dead fibers.... ev/select still revives fibers.... Feb 20, 2024
@bakpakin
Copy link
Member

Hard to say for certain yet (I'm looking into this) but I think the issue is with channel/from-each looking for specific errors and then calling propagate.

@bakpakin bakpakin added the bug This is not expected behavior, and needs fixing label Feb 21, 2024
@amano-kenji
Copy link
Contributor Author

I'm thinking that I'm using ev/select wrong. In the above example, ev/select is called twice on the same channel.

@bakpakin
Copy link
Member

So on latest master, this seems to work as expected (as I expected). I wrapped every call to ev/go and ev/spawn with tracev so we can see which fibers are created, and then added this bit of code to monitor which fibers are active:

(forever
  (ev/sleep 1)
  (def all-tasks (ev/all-tasks))
  (def me (fiber/root))
  (def sans-self (seq [t :in all-tasks :when (not= me t)] t))
  (printf "%n" sans-self))

At the end, a single fiber is left running that corresponds to the loop that is reading from the supervisor channel via (ev/spawn (forever ...))

Closing as cannot reproduce, please reopen if still an issue.

@bakpakin bakpakin closed this as not planned Won't fix, can't repro, duplicate, stale May 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This is not expected behavior, and needs fixing
Projects
None yet
Development

No branches or pull requests

2 participants