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/deadline and ev/with-deadline should be documented better. #1349

Open
amano-kenji opened this issue Jan 4, 2024 · 6 comments
Open

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

amano-kenji opened this issue Jan 4, 2024 · 6 comments
Assignees

Comments

@amano-kenji
Copy link
Contributor

amano-kenji commented Jan 4, 2024

  • In (ev/with-deadline deadline & body), deadline should become sec.
  • In (ev/deadline sec &opt tocancel tocheck), I don't know the difference between tocancel and tocheck. If deadline is reached, what exactly happens to tocheck and tocancel?
  • ev/deadline throws an error if deadline is reached. This behavior is not documented.
@sogaiu
Copy link
Contributor

sogaiu commented Jan 4, 2024

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:

  • tocancel apparently defaults to (fiber/root) (janet_vm.root_fiber), and in the above .c code corresponds to to.fiber
  • tocheck apparently defaults to (fiber/current)
    (janet_vm.fiber), and in the above .c code corresponds to to.curr_fiber

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;

@amano-kenji amano-kenji changed the title ev/deadline and ev/with-deadlin should be documented better. ev/deadline and ev/with-deadline should be documented better. Jan 4, 2024
@bakpakin bakpakin self-assigned this Jan 18, 2024
@sogaiu
Copy link
Contributor

sogaiu commented Feb 13, 2024

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
Copy link
Contributor

sogaiu commented Feb 18, 2024

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
Copy link
Contributor

sogaiu commented Feb 18, 2024

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
Copy link
Contributor

sogaiu commented Feb 18, 2024

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
Copy link
Contributor

sogaiu commented Feb 20, 2024

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.

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

No branches or pull requests

3 participants