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

RFC / Feature request: cancelable #782

Closed
raphael-proust opened this issue May 14, 2020 · 2 comments · Fixed by #785
Closed

RFC / Feature request: cancelable #782

raphael-proust opened this issue May 14, 2020 · 2 comments · Fixed by #785

Comments

@raphael-proust
Copy link
Collaborator

Whilst coding I stumbled upon a situation where I needed a promise to be reliably cancelable. As in Lwt.cancel p needed to cause p to be rejected with Canceled if p was in the Sleep state. I came up with the following:

let cancelable p =
   match Lwt.state p with
   | Return _ | Fail _ -> p
   | Sleep -> Lwt.pick [p; fst (Lwt.task ())]
  1. (RFC) This does what I need, but: have I missed some corner cases?

  2. (Feature request) This would live nicely along protected and no_cancel in the Lwt module. I'm open to a change of name because cancelable could also be the name of a predicate to test whether a promise is cancelable. Maybe unprotected (but it suggests that this cancels out protected) or always_cancelable (but it's not if the argument is resolved)?


Considering protected, no_cancel, and cancelable suggests a fourth function: one that propagates the cancelation search backwards but doesn't propagate the cancelation error forwards. This completes the cross product of “carries cancelation backwards”/“doesn't” with “fails with Canceled when canceled”/“doesn't”.

Something along the lines of

let hide_cancelation p =
  Lwt.catch
    (fun () -> p)
    (function Lwt.Canceled -> fst @@ Lwt.wait () | e -> Lwt.fail e)

Although I can't find uses for it right now, and returning a never-resolving promise is a bit strange.

@aantron
Copy link
Collaborator

aantron commented May 14, 2020

  1. have I missed some corner cases?

None come to mind, though it does seem like a scary way to implement the function :) I would have probably used Lwt.task, Lwt.on_any, and Lwt.on_cancel to directly tie two promises together, rather than introduce a third promise with Lwt.pick.

I'd merge a PR for this function, but implemented in terms of the internals of Lwt (see protected and no_cancel). I can't readily think of a good name for it, either :)

hide_cancelation has both potentially dangerous behavior (the forever-pending promise) and known use case, so I suggest not to add it now.

@raphael-proust
Copy link
Collaborator Author

I will prepare an MR.

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

Successfully merging a pull request may close this issue.

2 participants