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.

Sign up for GitHub

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

Jump to bottom

Safe Fiber races: ~combine and Fiber.n_any #587

Merged
merged 1 commit into from
Jan 3, 2024
Merged

Safe Fiber races: ~combine and Fiber.n_any #587

merged 1 commit into from
Jan 3, 2024

Conversation

SGrondin
Copy link
Collaborator

@SGrondin SGrondin commented Jul 19, 2023 •
edited
Loading

Addresses #558

I originally tried a much more complex approach before realizing the obvious one is probably good enough.

I tried to validate that this solution is sufficient by reading how fibers work internally, but I wouldn't be surprised if I missed some important detail.

The user-facing API turned out quite nice, in my opinion. Thanks for the suggestion in #558 @talex5

This PR is a prerequisite for #584

@SGrondin SGrondin mentioned this pull request Jul 20, 2023
Closed
@SGrondin
Copy link
Collaborator Author

SGrondin commented Nov 7, 2023

@talex5 Even though this is not needed for the Executor_pool anymore, I still think it should be considered for merging. I have Lwt programs that cannot be converted to Eio without it.

talex5
talex5 reviewed Jan 2, 2024
View reviewed changes
Copy link
Collaborator

@talex5 talex5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I've pushed a commit that combines the results as it goes rather than reversing the list at the end. It makes the code a bit shorter.

lib_eio/core/eio__core.mli Show resolved Hide resolved
@talex5 talex5 force-pushed the fiber-races branch 2 times, most recently from 2ff38fd to 7b932ef Compare January 3, 2024 10:42
talex5
talex5 approved these changes Jan 3, 2024
View reviewed changes
Copy link
Collaborator

@talex5 talex5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Squashed with some minor doc and test changes - thanks!

@talex5 talex5 merged commit d338f8b into ocaml-multicore:main Jan 3, 2024
3 of 5 checks passed
@SGrondin SGrondin deleted the fiber-races branch January 3, 2024 14:36
@SGrondin SGrondin mentioned this pull request Jan 3, 2024
Closed
talex5 added a commit to talex5/opam-repository that referenced this pull request Jan 17, 2024
@talex5
[new release] eio (5 packages) (0.14)
5340789 
CHANGES:

New features / API changes:

- Add `Eio.Executor_pool` (@SGrondin ocaml-multicore/eio#639, reviewed by @talex5).
  Provides an easy way to distribute jobs across domains.

- Add `Fiber.first ~combine` and `Fiber.n_any` (@SGrondin @talex5 ocaml-multicore/eio#587).
  Allows keeping both results in the case where multiple fibers succeed.

- Add `Eio_mock.Backend.run_full` with auto-advancing mock clock (@talex5 ocaml-multicore/eio#644, reviewed by @SGrondin).
  Simplifies testing of code using clocks.

- Add `Buf_write.printf` (@SGrondin @talex5 ocaml-multicore/eio#655).

- Add `Net.listening_addr` (@mefyl ocaml-multicore/eio#555, reviewed by @patricoferris @talex5).
  Useful to get the socket's address if the OS assigns it.

- Add `Promise.try_resolve` (@talex5 ocaml-multicore/eio#646).

- Remove `Cancel_hook_failed` exception (@talex5 ocaml-multicore/eio#640).
  Didn't seem to be used and broke dscheck.

Tracing:

- Improve tracing (@TheLortex @patricoferris @talex5 ocaml-multicore/eio#656).
  Trace cancellation contexts and OS operations, and simplify API.

- Add labels to switches (@talex5 ocaml-multicore/eio#661, reviewed by @SGrondin).

- `Fiber.all`: use the parent fiber (@talex5 ocaml-multicore/eio#665, reviewed by @SGrondin).
  Cleans up the traces a bit.

Performance:

- Faster and simpler `Lf_queue` (@talex5 ocaml-multicore/eio#647, based on work by @polytypic).

- Optimise `Flow.copy` with `Buf_read.as_flow` (@talex5 ocaml-multicore/eio#663, reviewed by @SGrondin, reported by @leostera).

Bug fixes:

- Fix handling of very long IO vectors (@talex5 ocaml-multicore/eio#653, reported by @Cjen1).

- eio_posix: use `caml_enter_blocking_section` in more places (@talex5 ocaml-multicore/eio#654, reviewed by @SGrondin).

- eio_posix: work around `caml_unix_alloc_sockaddr` bug (@talex5 ocaml-multicore/eio#651).

- Remove default backtrace from `Switch.fail` (@talex5 ocaml-multicore/eio#664).

Documentation:

- Organise eio.mli better (@talex5 ocaml-multicore/eio#667).

- Fix quoting of quotes in process error messages (@talex5 ocaml-multicore/eio#666, reviewed by @SGrondin).

- Mention Path module in File and Fs documentation (@talex5 ocaml-multicore/eio#659, requested by @clecat).

- Minor documentation updates (@SGrondin @talex5 ocaml-multicore/eio#670).

Build / internals:

- Allow closing synchronous streams (@talex5 ocaml-multicore/eio#641, reviewed by @SGrondin).
  This isn't currently exposed in the public interface.

- Fix non-idempotent tests (@SGrondin ocaml-multicore/eio#662).

- eio_windows: add explicit fmt dependency (@talex5 ocaml-multicore/eio#643).
@talex5 talex5 mentioned this pull request Jan 17, 2024
Merged
talex5 added a commit to talex5/opam-repository that referenced this pull request Jan 17, 2024
@talex5
[new release] eio (5 packages) (0.14)
6eb34f3 
CHANGES:

New features / API changes:

- Add `Eio.Executor_pool` (@SGrondin ocaml-multicore/eio#639, reviewed by @talex5).
  Provides an easy way to distribute jobs across domains.

- Add `Fiber.first ~combine` and `Fiber.n_any` (@SGrondin @talex5 ocaml-multicore/eio#587).
  Allows keeping both results in the case where multiple fibers succeed.

- Add `Eio_mock.Backend.run_full` with auto-advancing mock clock (@talex5 ocaml-multicore/eio#644, reviewed by @SGrondin).
  Simplifies testing of code using clocks.

- Add `Buf_write.printf` (@SGrondin @talex5 ocaml-multicore/eio#655).

- Add `Net.listening_addr` (@mefyl ocaml-multicore/eio#555, reviewed by @patricoferris @talex5).
  Useful to get the socket's address if the OS assigns it.

- Add `Promise.try_resolve` (@talex5 ocaml-multicore/eio#646).

- Remove `Cancel_hook_failed` exception (@talex5 ocaml-multicore/eio#640).
  Didn't seem to be used and broke dscheck.

Tracing:

- Improve tracing (@TheLortex @patricoferris @talex5 ocaml-multicore/eio#656).
  Trace cancellation contexts and OS operations, and simplify API.

- Add labels to switches (@talex5 ocaml-multicore/eio#661, reviewed by @SGrondin).

- `Fiber.all`: use the parent fiber (@talex5 ocaml-multicore/eio#665, reviewed by @SGrondin).
  Cleans up the traces a bit.

Performance:

- Faster and simpler `Lf_queue` (@talex5 ocaml-multicore/eio#647, based on work by @polytypic).

- Optimise `Flow.copy` with `Buf_read.as_flow` (@talex5 ocaml-multicore/eio#663, reviewed by @SGrondin, reported by @leostera).

Bug fixes:

- Fix handling of very long IO vectors (@talex5 ocaml-multicore/eio#653, reported by @Cjen1).

- eio_posix: use `caml_enter_blocking_section` in more places (@talex5 ocaml-multicore/eio#654, reviewed by @SGrondin).

- eio_posix: work around `caml_unix_alloc_sockaddr` bug (@talex5 ocaml-multicore/eio#651).

- Remove default backtrace from `Switch.fail` (@talex5 ocaml-multicore/eio#664).

Documentation:

- Organise eio.mli better (@talex5 ocaml-multicore/eio#667).

- Fix quoting of quotes in process error messages (@talex5 ocaml-multicore/eio#666, reviewed by @SGrondin).

- Mention Path module in File and Fs documentation (@talex5 ocaml-multicore/eio#659, requested by @clecat).

- Minor documentation updates (@SGrondin @talex5 ocaml-multicore/eio#670).

Build / internals:

- Allow closing synchronous streams (@talex5 ocaml-multicore/eio#641, reviewed by @SGrondin).
  This isn't currently exposed in the public interface.

- Fix non-idempotent tests (@SGrondin ocaml-multicore/eio#662).

- eio_windows: add explicit fmt dependency (@talex5 ocaml-multicore/eio#643).
nberth pushed a commit to nberth/opam-repository that referenced this pull request Jun 18, 2024
@talex5 @nberth
[new release] eio (5 packages) (0.14)
c39875a 
CHANGES:

New features / API changes:

- Add `Eio.Executor_pool` (@SGrondin ocaml-multicore/eio#639, reviewed by @talex5).
  Provides an easy way to distribute jobs across domains.

- Add `Fiber.first ~combine` and `Fiber.n_any` (@SGrondin @talex5 ocaml-multicore/eio#587).
  Allows keeping both results in the case where multiple fibers succeed.

- Add `Eio_mock.Backend.run_full` with auto-advancing mock clock (@talex5 ocaml-multicore/eio#644, reviewed by @SGrondin).
  Simplifies testing of code using clocks.

- Add `Buf_write.printf` (@SGrondin @talex5 ocaml-multicore/eio#655).

- Add `Net.listening_addr` (@mefyl ocaml-multicore/eio#555, reviewed by @patricoferris @talex5).
  Useful to get the socket's address if the OS assigns it.

- Add `Promise.try_resolve` (@talex5 ocaml-multicore/eio#646).

- Remove `Cancel_hook_failed` exception (@talex5 ocaml-multicore/eio#640).
  Didn't seem to be used and broke dscheck.

Tracing:

- Improve tracing (@TheLortex @patricoferris @talex5 ocaml-multicore/eio#656).
  Trace cancellation contexts and OS operations, and simplify API.

- Add labels to switches (@talex5 ocaml-multicore/eio#661, reviewed by @SGrondin).

- `Fiber.all`: use the parent fiber (@talex5 ocaml-multicore/eio#665, reviewed by @SGrondin).
  Cleans up the traces a bit.

Performance:

- Faster and simpler `Lf_queue` (@talex5 ocaml-multicore/eio#647, based on work by @polytypic).

- Optimise `Flow.copy` with `Buf_read.as_flow` (@talex5 ocaml-multicore/eio#663, reviewed by @SGrondin, reported by @leostera).

Bug fixes:

- Fix handling of very long IO vectors (@talex5 ocaml-multicore/eio#653, reported by @Cjen1).

- eio_posix: use `caml_enter_blocking_section` in more places (@talex5 ocaml-multicore/eio#654, reviewed by @SGrondin).

- eio_posix: work around `caml_unix_alloc_sockaddr` bug (@talex5 ocaml-multicore/eio#651).

- Remove default backtrace from `Switch.fail` (@talex5 ocaml-multicore/eio#664).

Documentation:

- Organise eio.mli better (@talex5 ocaml-multicore/eio#667).

- Fix quoting of quotes in process error messages (@talex5 ocaml-multicore/eio#666, reviewed by @SGrondin).

- Mention Path module in File and Fs documentation (@talex5 ocaml-multicore/eio#659, requested by @clecat).

- Minor documentation updates (@SGrondin @talex5 ocaml-multicore/eio#670).

Build / internals:

- Allow closing synchronous streams (@talex5 ocaml-multicore/eio#641, reviewed by @SGrondin).
  This isn't currently exposed in the public interface.

- Fix non-idempotent tests (@SGrondin ocaml-multicore/eio#662).

- eio_windows: add explicit fmt dependency (@talex5 ocaml-multicore/eio#643).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@talex5 talex5 talex5 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@SGrondin @talex5