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

Semantics of Lwt.pick / choose #856

Closed
cyberhuman opened this issue May 21, 2021 · 1 comment · Fixed by #874
Closed

Semantics of Lwt.pick / choose #856

cyberhuman opened this issue May 21, 2021 · 1 comment · Fixed by #874
Labels

Comments

@cyberhuman
Copy link

The documentation for Lwt.pick says:

It's possible for multiple promises in ps to become resolved simultaneously. This happens most often when some promises ps are already resolved at the time Lwt.pick is called.

In that case, if at least one of the promises is rejected, the result promise p is rejected with the same exception as one such promise, chosen arbitrarily. If all promises are fulfilled, p is fulfilled with the value of one of the promises, also chosen arbitrarily.

However, the first sentence in the second paragraph isn't true, judging by the code
https://github.com/ocsigen/lwt/blob/master/src/core/lwt.ml#L2753-L2755

a simple test confirms this:

let rec loop (ok, err) = function
  | n when n <= 0 -> Lwt_io.printlf "ok %d err %d" ok err
  | n ->
  let%lwt res =
    let exception Err of (int * int) in
    try%lwt Lwt.pick [ Lwt.return (succ ok, err); Lwt.fail (Err (ok, succ err)); ] with Err x -> Lwt.return x
  in
  loop res (pred n)
in
loop (0, 0) 10;;
ok 5 err 5
@raphael-proust
Copy link
Collaborator

Hello,

Your diagnostic is correct. In fact, when multiple promises are already resolved, pick simply takes a random one of them. It doesn't special case whether some of the already resolved promises are rejected or not.

I'll prepare an MR to fix that.

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

Successfully merging a pull request may close this issue.

3 participants