janet-lang / janet

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

`ev/deadline` and `ev/with-deadline` should be documented better. #1349

Open amano-kenji opened 6 months ago

amano-kenji commented 6 months ago
sogaiu commented 6 months ago

Not an answer, but may be some relevant code:

    while (peek_timeout(&to) && to.when <= now) {
        pop_timeout(0);
        if (to.curr_fiber != NULL) {
            if (janet_fiber_can_resume(to.curr_fiber)) {
                janet_cancel(to.fiber, janet_cstringv("deadline expired"));
            }
        } else {
            /* This is a timeout (for a function call, not a whole fiber) */
            if (to.fiber->sched_id == to.sched_id) {
                if (to.is_error) {
                    janet_cancel(to.fiber, janet_cstringv("timeout"));
                } else {
                    janet_schedule(to.fiber, janet_wrap_nil());
                }
            }
        }
    }

Some further bits:

Some info about janet_vm.root_fiber and janet_vm.fiber via the definition of JanetVM in state.h:

    /* The current running fiber on the current thread.
     * Set and unset by functions in vm.c */
    JanetFiber *fiber;
    JanetFiber *root_fiber;
sogaiu commented 5 months ago

For reference, I've started to gather some more details about ev/deadline and ev/with-deadline here.

Once I've gotten a better understanding, I'm willing to work on addressing this issue.

sogaiu commented 4 months ago

Ok, I've looked into this a bit more and have some results to share. (Note: I'm not done, this is a status update.)

I'll start with sample code:

(def check-wait 1.1)
(def cancel-wait (* 2 check-wait))
(def deadline (* 0.5 check-wait))

(var tocancel-fib nil)
(var tocheck-fib nil)

(set tocancel-fib
  (ev/go
    (fiber/new
      (fn []
        (print "tocancel: started")
        (set tocheck-fib
             (fiber/new
               (fn []
                 (print "tocheck: started")
                 (printf "tocheck: waiting: %n sec" check-wait)
                 (ev/sleep check-wait)
                 (print "tocheck: ended"))))
        (resume tocheck-fib)
        (printf "tocancel: waiting: %n sec" cancel-wait)
        (ev/sleep cancel-wait)
        (print "tocancel: ended")))))

# yield so tocancel-fib can start
(ev/sleep 0)

(ev/deadline deadline tocancel-fib tocheck-fib)

Output looked like:

tocancel: started
tocheck: started
tocheck: waiting: 1.1 sec
error: deadline expired
  in ev/sleep [src/core/ev.c] on line 2938
  in <anonymous> [ev-deadline.janet] on line 18, column 18
  in <anonymous> [ev-deadline.janet] on line 20, column 9

It may or may not be obvious from the above, but I think that one thing it demonstrates is that tocancel should be a task (a fiber managed by the event loop) and tocheck can be an ordinary fiber.

It wasn't obvious to me initially from the docstring, but now that I think I understand more I see that this bit of text in the docstring:

tocancel will be canceled as with ev/cancel

implies that it must be a task (because non-task fibers are not canceled with ev/cancel, but rather cancel).

My current leaning is that it might be better to have text along the lines of:

Note that if tocancel is given, it must be a task / root fiber.

or:

Note that if tocancel is given, it must already be on the event loop.

immediately after:

tocancel will be canceled as with ev/cancel.

sogaiu commented 4 months ago

As to what happens to tocancel and tocheck after the deadline is "triggered" by the event loop, as I understand it, tocancel is "canceled" using janet_cancel (this is also the function that ev/cancel is built on top of IIUC).

So perhaps it's fair to claim that the current docstring says things along those lines:

tocancel will be canceled as with ev/cancel.

As far as tocheck is concerned, I haven't reached a view, but I'm working on it (^^;

My current guess is that it may depend on its relationship to tocancel.

If, as in the sample code above, tocheck is a fiber arranged for from "within" tocancel, perhaps tocheck will be "taken care of" in the process of canceling tocancel. Not sure though.


As far as ev/deadline is concerned, may be it's the case that it doesn't do anything directly to tocheck. Perhaps it only attempts to cancel tocancel using the underlying bits of ev/cancel.

sogaiu commented 4 months ago

Regarding:

ev/deadline throws an error if deadline is reached. This behavior is not documented.

I was looking at the docstrings for cancel:

Resume a fiber but have it immediately raise an error. This lets a programmer unwind a pending fiber. Returns the same result as resume.

and ev/cancel:

Cancel a suspended fiber in the event loop. Differs from cancel in that it returns the canceled fiber immediately.

I think if the deadline is reached, then ev/cancel (or equivalent underlying C) is used on tocancel:

If tocheck is not finished after sec seconds, tocancel will be canceled as with ev/cancel.

If ev/cancel is like cancel and cancel raises an error, then the current behavior doesn't seem surprising to me:

(def tocancel
  (ev/go (coro (yield 1))))

# let `tocancel` start by yielding to the event loop
(ev/sleep 0.1)

# try to cancel `tocancel`
(ev/cancel tocancel nil)

# sample output
#
# error: 
#   in <anonymous> [ev-cancel.janet] on line 2, column 16

ev/deadline's docstring does mention ev/cancel and ev/cancel's docstring does mention cancel.

It did take some looking things up and combining together, but in this case aren't all lookups within docstrings?


Note that the website docs do contain coverage of cancellation wrt the event loop. There is even sample code there:

(def f
  (ev/spawn
    (print "starting long io...")
    (ev/sleep 10000)
    (print "finished long io!")))

# wait 2 seconds before canceling the long IO.
(ev/sleep 2)
(ev/cancel f "canceled")

When I put this code in a file (say ev-cancel.janet) and run it, I currently get:

starting long io...
error: canceled
  in ev/sleep [src/core/ev.c] on line 2938
  in _spawn [ev-cancel.janet] on line 4, column 5

There is even accompanying text:

Sometimes, IO operations can take too long or even hang indefinitely. Janet offers ev/cancel to interrupt and cancel an ongoing IO operation. This will cause the canceled task to be resumed but immediately error. From the point of view of the canceled task, it will look as though the last function that yielded to the event loop raised an error.

sogaiu commented 4 months ago

Based on looking into this matter with @llmII (and some of the original suggestions from @amano-kenji), the following text includes some modifications to the two docstrings in question. I'm thinking to make a PR based on the content.


(ev/with-deadline sec & body)

Run a body of code with a deadline of sec seconds, such that if the code does not complete before the deadline is up, it will be canceled.

sec is a number that can have a fractional part.


(ev/deadline sec &opt tocancel tocheck)

Set a deadline for a fiber tocheck.

If tocheck is not finished after sec seconds, tocancel will be canceled as with ev/cancel. Note that if tocancel is given, it must be a task (aka root fiber).

sec is a number that can have a fractional part.

If tocancel and tocheck are not given, they default to (fiber/root) and (fiber/current) respectively.

Returns tocancel.


With respect to what happens to tocheck, AFAIU (from reading docs, reading code, and carrying out experiments), my current view is that ev/deadline does not do anything directly to tocheck.

IIUC, the caller of ev/deadline can set things up so that if the deadline is reached and tocancel is canceled, the tocancel fiber can "catch" this and try to do something with tocheck if it wants. I think this is up to the programmar to arrange and it is general fiber know-how, i.e. not particular to ev/deadline [1].


Please see this comment regarding:

ev/deadline throws an error if deadline is reached. This behavior is not documented.

I believe I have addressed it in that comment.


[1] Though I have made a note that it might be good to add this kind of info to the website docs, possibly near or on the fiber page.