janet-lang / janet

A dynamic language and bytecode vm
https://janet-lang.org
MIT License
3.38k stars 217 forks source link

`ev/select` should not resume a dead task. #1405

Closed amano-kenji closed 4 months ago

amano-kenji commented 4 months ago
(def ch (ev/chan))
(def ch1 (ev/chan))

(def task (ev/spawn
            (defer (do
                     (print "exiting task")
                     (:close ch1))
              (forever
                (print 3)
                (ev/sleep 1)))))

(ev/sleep 2)

(ev/spawn
  (match (ev/select ch ch1)
    [:close _]
    (do
      (ev/cancel task :cancel)
      (error "error after calling ev/cancel"))))

(ev/sleep 1)
(:close ch)

(forever
  (ev/sleep 1))

ev/select tries to resume the second task that errored out because ch1 is closed later in another task.

ev/select should not revive a dead task. ev/select effectively turns a task into an undead task. I don't like zombies.

amano-kenji commented 4 months ago

It seems that once I call ev/select on channels, those channels become unuseable with ev/take or ev/give.

My suspicion is that janet_chan_unlock is not called for all the given channels once a channel is closed, receives a value, or gives a value.

bakpakin commented 4 months ago

It seems that once I call ev/select on channels, those channels become unuseable with ev/take or ev/give.

That seems like a separate issue, but also seems incorrect. For example:

(def c (ev/chan 10))
(def d (ev/chan 10))

(ev/spawn
  (forever
    (match (ev/select c d)
      [:close x]
      (print "channel " x " closed")
      [:take x y]
      (print "got " y " from " x))))

(forever
  (ev/sleep 1)
  (if (> (math/random) 0.5)
    (ev/give c :item)
    (ev/give d :item)))

this works for me

amano-kenji commented 4 months ago

I'll come back to this tomorrow.

amano-kenji commented 4 months ago

I just tested the latest commit. This issue is fixed, but I may discover more issues with ev/select soon.