-
Notifications
You must be signed in to change notification settings - Fork 67
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
Initial Eio_posix backend #448
Conversation
🐮 moo I'm back today, I did release a iomux 0.1 just before leaving, but that won't work on macos cause, believe it or not, it doens't have ppoll(2). I'll make a new release today that makes it fallback nicely to poll(2) on macos. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great and is mostly working on macOS (M1, 13.2)! Apart from the inline comments, in the README there is a little test for copying from stdin to stdout:
let () =
Eio_main.run @@ fun env ->
Eio.Flow.copy
(Eio.Stdenv.stdin env)
(Eio.Stdenv.stdout env);;
this is currently making MDX hang when testing the README.
do { | ||
void *buf = Caml_ba_data_val(v_ba) + off; | ||
caml_enter_blocking_section(); | ||
ret = getrandom(buf, len, 0); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
getrandom
is not available on macOS it would seem, there's getentropy
and arc4random
which look like they may be a good fit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually I think arc4random_buf is preferred, I thought it wasn't very portable but it's everywhere we want apparently.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've updated it to use arc4random_buf
on non-Linux systems now.
lib_eio_posix/low_level.ml
Outdated
| Unix.Unix_error ((EAGAIN | EWOULDBLOCK | EALREADY | EINPROGRESS), _, _) -> | ||
await_writable fd; | ||
match Fd.use_exn "connect" fd Unix.getsockopt_error with | ||
| None -> connect fd addr |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This part, on macOS, is failing the networking tests. If I understand it correctly, wouldn't an EINPROGRESS
plus waiting for the FD to be writeable mean by the time we get to the match statement the error would be cleared and the socket would be connected meaning this second connect would fail with:
Eio.Io Unix_error (Socket is already connected, "connect", "")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're correct, I think the test is wrong. There shouldn't be a second connect, if SO_ERROR is cleared then it's all done and there's nothing else to do (says Mr.Stevens).
It probably only works on the linux case because a local connection is allowed to never return EINPROGRESS and finish immediately.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense (the man-page is a bit vague about it). What about the other cases (EAGAIN | EWOULDBLOCK | EALREADY
)?
The Linux man-page says:
EINPROGRESS The socket is nonblocking and the connection cannot be completed immediately. (UNIX domain sockets failed with EAGAIN instead.) [ then talks about waiting and checking for errors ]
But the EAGAIN case doesn't say what you should do in that case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a TCP socket you shouldn't get EAGAIN/EWOULDBLOCK (IIRC you can get on an AF_UNIX/SOCKSTREAM).
Since now you don't issue a secondary connect you should also never see an EALREADY.
But, I think you're missing the EINTR case, which should be handled the same way as a blocking, which means EINTR does not cancel a connect so you should wait for writeable and check the error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK. Since we're always nonblocking, I guess we shouldn't normally see EINTR, but I'll treat it the same anyway.
lib_eio_posix/low_level.ml
Outdated
@@ -60,10 +60,11 @@ let rec connect fd addr = | |||
with | |||
| Unix.Unix_error (EINTR, _, _) -> connect fd addr (* Just in case *) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This EINTR should be handled like EINPROGRESS below, and I think you can zap EAGAIN | EWOULDBLOCK | EALREADY
2b62338
to
4c1fadb
Compare
Co-authored-by: Christiano Haesbaert <haesbaert@haesbaert.org>
I guess stdin isn't |
@haesbaert I made some minor changes to the iomux bits, which looked safe to me:
|
If e.g. `socketpair` fails, we now return the exception to the calling fiber rather than aborting the event loop. Also, attach FDs to switches before setting non-blocking mode, in case that fails (requested by Anil Madhavapeddy).
Ack, I've changed it to invalidate always, that was just way too dangerous. |
I had accidentally force-pushed here, I removed, I'm keeping my additional changes here: |
Note that MDX no longer requires this (see realworldocaml/mdx#398).
It's only one place (
I don't see any commits there. It's OK to push to this branch (no need to force-push). We can squash before merging if needed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So this is a partial review, I'm still missing the harder bits, but this is a bit too much to do in one go.
lib_eio_posix/eio_posix_stubs.c
Outdated
CAMLprim value caml_eio_posix_renameat(value v_old_fd, value v_old_path, value v_new_fd, value v_new_path) { | ||
CAMLparam2(v_old_path, v_new_path); | ||
int old_path_len = caml_string_length(v_old_path); | ||
int new_path_len = caml_string_length(v_new_path); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
size_t would be best here as the runtime uses a mlsize_t
int ret; | ||
caml_unix_check_path(v_old_path, "renameat-old"); | ||
caml_unix_check_path(v_new_path, "renameat-new"); | ||
old_path = caml_stat_alloc(old_path_len + new_path_len + 2); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I understand you over-copy String_val expecting the last byte+1 to be null, like caml_stat_strdup_noexc
does, it took me some time to realize this code is actually correct as I couldn't find any guarantees that it's safe to assume this.
I'd be more comfortable with two calls to caml_stat_strdup
and one extra allocation, people copy code and things shuffle and they introduce bugs. Just noting this here as note of maintainability, feel free to ignore.
As a bonus it would be consistent by adding the caml_unix_check_path
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find any guarantees that it's safe to assume this.
https://v2.ocaml.org/manual/intfc.html#ss:c-block-access says:
String_val(v) returns a pointer to the first byte of the string v, with type const char *. This pointer is a valid C string: there is a null byte after the last byte in the string.
BTW, the way it does this is quite interesting. The block header just stores the number of words, so for strings it uses the last byte in the block as the number of '\0'
padding bytes, the first of which acts as the C terminator. If there are no padding bytes, then the "zero padding bytes" byte acts as the terminator!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd be more comfortable with two calls to
caml_stat_strdup
Then if the second one fails we have to remember to free the first one. With a single allocation, we can just let it raise an exception and forget about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I couldn't find any guarantees that it's safe to assume this.
https://v2.ocaml.org/manual/intfc.html#ss:c-block-access says:
String_val(v) returns a pointer to the first byte of the string v, with type const char *. This pointer is a valid C string: there is a null byte after the last byte in the string.
BTW, the way it does this is quite interesting. The block header just stores the number of words, so for strings it uses the last byte in the block as the number of
'\0'
padding bytes, the first of which acts as the C terminator. If there are no padding bytes, then the "zero padding bytes" byte acts as the terminator!
I couldn't find any guarantees that it's safe to assume this.
https://v2.ocaml.org/manual/intfc.html#ss:c-block-access says:
Seems I have to improve my search manual foo, I was Ctrl+f String_val and somehow didn't catch that.
String_val(v) returns a pointer to the first byte of the string v, with type const char *. This pointer is a valid C string: there is a null byte after the last byte in the string.
BTW, the way it does this is quite interesting. The block header just stores the number of words, so for strings it uses the last byte in the block as the number of
'\0'
padding bytes, the first of which acts as the C terminator. If there are no padding bytes, then the "zero padding bytes" byte acts as the terminator!
let () = | ||
C.main ~name:"discover" (fun c -> | ||
let defs = | ||
C.C_define.import c ~c_flags:["-D_LARGEFILE64_SOURCE"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense to discover more things, in the stub for example you assume linux->getrandom else->arc4random. Should we discover these things like HAS_ARC4RANDOM and so on ? I don't want to bikeshed as the way it is builds and compiles where we need for now, but this will become a thing once we try to add eio-posix to windows and maybe some other obscure unix.
lib_eio_posix/eio_posix_stubs.c
Outdated
ssize_t off = (ssize_t)Long_val(v_off); | ||
ssize_t len = (ssize_t)Long_val(v_len); | ||
do { | ||
void *buf = Caml_ba_data_val(v_ba) + off; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
void *buf = Caml_ba_data_val(v_ba) + off; | |
void *buf = (uint8_t *)Caml_ba_data_val(v_ba) + off; |
lib_eio_posix/eio_posix_stubs.c
Outdated
value v_ba = Field(v_cs, 0); | ||
value v_off = Field(v_cs, 1); | ||
value v_len = Field(v_cs, 2); | ||
iov[i].iov_base = Caml_ba_data_val(v_ba) + Long_val(v_off); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
iov[i].iov_base = Caml_ba_data_val(v_ba) + Long_val(v_off); | |
iov[i].iov_base = (uint8_t *)Caml_ba_data_val(v_ba) + Long_val(v_off); |
let run main = | ||
(* SIGPIPE makes no sense in a modern application. *) | ||
Sys.(set_signal sigpipe Signal_ignore); | ||
Domain_mgr.run_event_loop main @@ object (_ : stdenv) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice that the main domain goes through the same path as the manager !
"mdx" {>= "1.10.0" & with-test} | ||
"eio_luv" {= version} | ||
"eio_linux" {= version & os = "linux"} | ||
"eio_posix" {= version & os != "windows"} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is there no os_type
here like dune ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
They're the ones from opam var
, so it doesn't look like it.
lib_eio_posix/domain_mgr.ml
Outdated
| Eio_unix.Private.Socketpair (sw, domain, ty, protocol) -> Some (fun k -> | ||
let a, b = Unix.socketpair ~cloexec:true domain ty protocol in | ||
Unix.set_nonblock a; | ||
Unix.set_nonblock b; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just did a quick check in the OpenBSD kernel, I'd say it's pretty safe to assume you can expect set_nonblock to be ok, as long as we know what the file descriptor is. There are some weird cases like trying to set_nonblock on a kqueue fd will fail since there is no ioctl handler.
} | ||
with Unix.Unix_error (code, name, arg) -> raise @@ Err.wrap code name arg | ||
|
||
let write_bufs fd bufs = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not particular to this change but I realize we might want to consider exporting a single_write
like single_read
, the reason for that is that if any of the Flow write fails, the user has no way to know how many bytes have been copied before getting the exception.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes; feel free to open a separate issue for that (it affects all backends).
(* stdin, stdout and stderr are blocking, and so need special care. | ||
For these, we first wait for them to become e.g. readable and then hope | ||
that the read doesn't block. This may fail if multiple fibers try to read | ||
at the same time. We could check that it's still readable after being | ||
resumed, but that still won't work if multiple domains read at the same | ||
time. Same problem for writes. *) | ||
blocking : bool; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be worth considering making stdin nonblocking, and being careful to restore the blockiness on fork.
This way at least we guarantee it's safe (even if silly) to have two fibers waiting on input.
I'd not touch stderr or stdout, because users will still end up using Printf directly, but stdin we might consider assuming "this is ours". At any rate would be nice to see what others are doing, maybe the nonblock->restore dance ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lwt does the same thing (waits and then does the read): https://github.com/ocsigen/lwt/blob/cc05e2bda6c34126a3fd8d150ee7cddb3b8a440b/src/unix/lwt_unix.cppo.ml#L656
But it does it in a systhread, so it's not a big problem. We can do the same once eio_posix uses systhreads (after this initial PR is merged).
While here re-arrange include order, sys; std; caml.
With this tests pass in eio-posix for me.
We inflate maxi to account the highest index we're tracking, if we're about to unregister the highest one, find the new highest.
There was mention that some of this might/should also work on Windows. For testing on Windows, there is an example of how to use setup-ocaml to setup a working OCaml 5.0 Windows CI here. |
lib_eio_posix/eio_posix.ml
Outdated
Domain_mgr.run_event_loop main @@ object (_ : stdenv) | ||
method stdin = (Flow.of_fd Low_level.Fd.stdin :> <Eio.Flow.source; Eio_unix.unix_fd>) | ||
method stdout = (Flow.of_fd Low_level.Fd.stdout :> <Eio.Flow.sink; Eio_unix.unix_fd>) | ||
method stderr = (Flow.of_fd Low_level.Fd.stderr :> <Eio.Flow.sink; Eio_unix.unix_fd>) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it intentional / desirable that each call of stdin/stdout/stderr allocates a new flow object?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The wrapper is stateless (essentially it's just a tuple (flow_ops, fd)
), but I guess it would be more logical to return the same one each time. Ideally we'd use lazy
here, but that doesn't work on multicore.
I've pushed a commit that creates the wrappers once in all cases.
|
||
CAMLprim value caml_eio_posix_pwritev(value v_fd, value v_bufs, value v_offset) { | ||
CAMLparam2(v_bufs, v_offset); | ||
ssize_t r; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm… Is there a reason to prefer ancient C style with declarations at the beginning of a block?
I would recommend against that unless there is some really good reason for it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Historically C99 declarations didn't get that much adoption in the open source world (especially the UNIX world) because portability was important, I don't think this holds in 2023 and I'd be surprised if there is any issue. Having said that, I dislike declarations in the middle of a block, I like the discipline of having to declare beforehand.
so tldr: I think it's more tradition than anything else.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, let's keep style preferences out of the review (I do prefer the newer style, but most other OCaml code seems to be written this way).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The reason C code used to be written that way was that it was required (been there; done that).
Nowadays C supports having declarations anywhere that a statement can be used. This allows one to make it so that all variables are initialized with a proper value. It reduces the chance of making programming errors such as using uninitialized variables or having type mismatches (variable declared with one type then later initialized with another). It also promotes the use of more variables (rather than e.g. reusing a variable for multiple different purposes) and even using variables as in FP such that they are only initialized once and never written, which can further help to avoid programming errors. It also makes it easier to read the code as the scopes of variables can be narrower and be seen more clearly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we wanna catch more errors, which is always good, I'd suggest adding proper warnings to the compilation flags, the ones that dune select are absurdly spartan (as it's inherited from ocamlc_cflags), we basically build without any warningsenabled, standard flags are these:
ocamlc_cflags: -O2 -fno-strict-aliasing -fwrapv -pthread -fPIC
Doesn't matter too much, but might be useful if someone wants to compare them for equality.
It's only returned for `connect`, which does its own thing.
let write fd buf start len = | ||
if Fd.is_blocking fd then await_writable fd; | ||
Fd.use_exn "write" fd @@ fun fd -> | ||
do_nonblocking Write (fun fd -> Unix.write fd buf start len) fd | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you consider starvation ?
If you have a fiber that always reads/writes and it's always available, since now they don't even trap into the scheduler they can starve everything else.
In Oslo I was build an IO greed mechanism, basically you have to force them to yield either at every IO or at some other heuristic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it's one of the "Other things to work on after merging this initial support" items:
Ensure we yield on IO to prevent starvation.
I want to add the systhread stuff first, because that will fix some of the places by itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
lib_eio_posix/low_level.ml
Outdated
let rec do_nonblocking ty fn fd = | ||
try fn fd with | ||
| Unix.Unix_error (EINTR, _, _) -> do_nonblocking ty fn fd (* Just in case *) | ||
| Unix.Unix_error((EAGAIN | EWOULDBLOCK | EINPROGRESS), _, _) -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I don't think EINPROGRESS should be here, is it just defensive programming or I'm missing something else ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you're right - it's only useful for connect
, which has its own code for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this all looks fine.
CHANGES: New features: - Add eio_posix backend (@talex5 @haesbaert ocaml-multicore/eio#448 ocaml-multicore/eio#477, reviewed by @avsm @patricoferris @polytypic). This replaces eio_luv on all platforms except Windows (which will later switch to its own backend). It is a lot faster, provides access to more modern features (such as `openat`), and can safely share OS resources between domains. - Add subprocess support (@patricoferris @talex5 ocaml-multicore/eio#461 ocaml-multicore/eio#464 ocaml-multicore/eio#472, reviewed by @haesbaert @avsm). This is the low-level API support for eio_linux and eio_posix. A high-level cross-platform API will be added in the next release. - Add `Fiber.fork_seq` (@talex5 ocaml-multicore/eio#460, reviewed by @avsm). This is a light-weight alternative to using a single-producer, single-consumer, 0-capacity stream, similar to a Python generator function. Bug fixes: - eio_linux: make it safe to share FDs across domains (@talex5 ocaml-multicore/eio#440, reviewed by @haesbaert). It was previously not safe to share file descriptors between domains because if one domain used an FD just as another was closing it, and the FD got reused, then the original operation could act on the wrong file. - eio_linux: release uring if Linux is too old (@talex5 ocaml-multicore/eio#476). Avoids a small resource leak. - eio_linux: improve error handling creating pipes and sockets (@talex5 ocaml-multicore/eio#474, spotted by @avsm). If we get an error (e.g. too many FDs) then report it to the calling fiber, instead of exiting the event loop. - eio_linux: wait for uring to finish before exiting (@talex5 ocaml-multicore/eio#470, reviewed by @avsm). If the main fiber raised an exception then it was possible to exit while a cancellation operation was still in progress. - eio_main: make `EIO_BACKEND` handling more uniform (@talex5 ocaml-multicore/eio#447). Previously this environment variable was only used on Linux. Now all platforms check it. - Tell dune about `EIO_BACKEND` (@talex5 ocaml-multicore/eio#442). If this changes, dune needs to re-run the tests. - eio_linux: add some missing close-on-execs (@talex5 ocaml-multicore/eio#441). - eio_linux: `read_exactly` fails to update file offset (@talex5 ocaml-multicore/eio#438). - Work around dune `enabled_if` bug on non-Linux systems (@polytypic ocaml-multicore/eio#475, reviewed by @talex5). - Use raw system call of `getrandom` for glibc versions before 2.25 (@zenfey ocaml-multicore/eio#482). Documentation: - Add `HACKING.md` with hints for working on Eio (@talex5 ocaml-multicore/eio#443, reviewed by @avsm @polytypic). - Improve worker pool example (@talex5 ocaml-multicore/eio#454). - Add more Conditions documentation (@talex5 ocaml-multicore/eio#436, reviewed by @haesbaert). This adds a discussion of conditions to the README and provides examples using them to handle signals. - Condition: fix the example in the docstring (@avsm ocaml-multicore/eio#468). Performance: - Add a network benchmark using an HTTP-like protocol (@talex5 ocaml-multicore/eio#478, reviewed by @avsm @patricoferris). - Add a benchmark for reading from `/dev/zero` (@talex5 ocaml-multicore/eio#439). Other changes: - Add CI for macOS (@talex5 ocaml-multicore/eio#452). - Add tests for `pread`, `pwrite` and `readdir` (@talex5 ocaml-multicore/eio#451). - eio_linux: split into multiple files (@talex5 ocaml-multicore/eio#465 ocaml-multicore/eio#466, reviewed by @avsm). - Update Dockerfile (@talex5 ocaml-multicore/eio#471). - Use dune.3.7.0 (@patricoferris ocaml-multicore/eio#457). - Mint exclusive IDs across domains (@TheLortex ocaml-multicore/eio#480, reported by @haesbaert, reviewed by @talex5). The tracing currently only works with a single domain anyway, but this will change when OCaml 5.1 is released.
CHANGES: New features: - Add eio_posix backend (@talex5 @haesbaert ocaml-multicore/eio#448 ocaml-multicore/eio#477, reviewed by @avsm @patricoferris @polytypic). This replaces eio_luv on all platforms except Windows (which will later switch to its own backend). It is a lot faster, provides access to more modern features (such as `openat`), and can safely share OS resources between domains. - Add subprocess support (@patricoferris @talex5 ocaml-multicore/eio#461 ocaml-multicore/eio#464 ocaml-multicore/eio#472, reviewed by @haesbaert @avsm). This is the low-level API support for eio_linux and eio_posix. A high-level cross-platform API will be added in the next release. - Add `Fiber.fork_seq` (@talex5 ocaml-multicore/eio#460, reviewed by @avsm). This is a light-weight alternative to using a single-producer, single-consumer, 0-capacity stream, similar to a Python generator function. Bug fixes: - eio_linux: make it safe to share FDs across domains (@talex5 ocaml-multicore/eio#440, reviewed by @haesbaert). It was previously not safe to share file descriptors between domains because if one domain used an FD just as another was closing it, and the FD got reused, then the original operation could act on the wrong file. - eio_linux: release uring if Linux is too old (@talex5 ocaml-multicore/eio#476). Avoids a small resource leak. - eio_linux: improve error handling creating pipes and sockets (@talex5 ocaml-multicore/eio#474, spotted by @avsm). If we get an error (e.g. too many FDs) then report it to the calling fiber, instead of exiting the event loop. - eio_linux: wait for uring to finish before exiting (@talex5 ocaml-multicore/eio#470, reviewed by @avsm). If the main fiber raised an exception then it was possible to exit while a cancellation operation was still in progress. - eio_main: make `EIO_BACKEND` handling more uniform (@talex5 ocaml-multicore/eio#447). Previously this environment variable was only used on Linux. Now all platforms check it. - Tell dune about `EIO_BACKEND` (@talex5 ocaml-multicore/eio#442). If this changes, dune needs to re-run the tests. - eio_linux: add some missing close-on-execs (@talex5 ocaml-multicore/eio#441). - eio_linux: `read_exactly` fails to update file offset (@talex5 ocaml-multicore/eio#438). - Work around dune `enabled_if` bug on non-Linux systems (@polytypic ocaml-multicore/eio#475, reviewed by @talex5). - Use raw system call of `getrandom` for glibc versions before 2.25 (@zenfey ocaml-multicore/eio#482). Documentation: - Add `HACKING.md` with hints for working on Eio (@talex5 ocaml-multicore/eio#443, reviewed by @avsm @polytypic). - Improve worker pool example (@talex5 ocaml-multicore/eio#454). - Add more Conditions documentation (@talex5 ocaml-multicore/eio#436, reviewed by @haesbaert). This adds a discussion of conditions to the README and provides examples using them to handle signals. - Condition: fix the example in the docstring (@avsm ocaml-multicore/eio#468). Performance: - Add a network benchmark using an HTTP-like protocol (@talex5 ocaml-multicore/eio#478, reviewed by @avsm @patricoferris). - Add a benchmark for reading from `/dev/zero` (@talex5 ocaml-multicore/eio#439). Other changes: - Add CI for macOS (@talex5 ocaml-multicore/eio#452). - Add tests for `pread`, `pwrite` and `readdir` (@talex5 ocaml-multicore/eio#451). - eio_linux: split into multiple files (@talex5 ocaml-multicore/eio#465 ocaml-multicore/eio#466, reviewed by @avsm). - Update Dockerfile (@talex5 ocaml-multicore/eio#471). - Use dune.3.7.0 (@patricoferris ocaml-multicore/eio#457). - Mint exclusive IDs across domains (@TheLortex ocaml-multicore/eio#480, reported by @haesbaert, reviewed by @talex5). The tracing currently only works with a single domain anyway, but this will change when OCaml 5.1 is released.
CHANGES: New features: - Add eio_posix backend (@talex5 @haesbaert ocaml-multicore/eio#448 ocaml-multicore/eio#477, reviewed by @avsm @patricoferris @polytypic). This replaces eio_luv on all platforms except Windows (which will later switch to its own backend). It is a lot faster, provides access to more modern features (such as `openat`), and can safely share OS resources between domains. - Add subprocess support (@patricoferris @talex5 ocaml-multicore/eio#461 ocaml-multicore/eio#464 ocaml-multicore/eio#472, reviewed by @haesbaert @avsm). This is the low-level API support for eio_linux and eio_posix. A high-level cross-platform API will be added in the next release. - Add `Fiber.fork_seq` (@talex5 ocaml-multicore/eio#460, reviewed by @avsm). This is a light-weight alternative to using a single-producer, single-consumer, 0-capacity stream, similar to a Python generator function. Bug fixes: - eio_linux: make it safe to share FDs across domains (@talex5 ocaml-multicore/eio#440, reviewed by @haesbaert). It was previously not safe to share file descriptors between domains because if one domain used an FD just as another was closing it, and the FD got reused, then the original operation could act on the wrong file. - eio_linux: release uring if Linux is too old (@talex5 ocaml-multicore/eio#476). Avoids a small resource leak. - eio_linux: improve error handling creating pipes and sockets (@talex5 ocaml-multicore/eio#474, spotted by @avsm). If we get an error (e.g. too many FDs) then report it to the calling fiber, instead of exiting the event loop. - eio_linux: wait for uring to finish before exiting (@talex5 ocaml-multicore/eio#470, reviewed by @avsm). If the main fiber raised an exception then it was possible to exit while a cancellation operation was still in progress. - eio_main: make `EIO_BACKEND` handling more uniform (@talex5 ocaml-multicore/eio#447). Previously this environment variable was only used on Linux. Now all platforms check it. - Tell dune about `EIO_BACKEND` (@talex5 ocaml-multicore/eio#442). If this changes, dune needs to re-run the tests. - eio_linux: add some missing close-on-execs (@talex5 ocaml-multicore/eio#441). - eio_linux: `read_exactly` fails to update file offset (@talex5 ocaml-multicore/eio#438). - Work around dune `enabled_if` bug on non-Linux systems (@polytypic ocaml-multicore/eio#475, reviewed by @talex5). - Use raw system call of `getrandom` for glibc versions before 2.25 (@zenfey ocaml-multicore/eio#482). Documentation: - Add `HACKING.md` with hints for working on Eio (@talex5 ocaml-multicore/eio#443, reviewed by @avsm @polytypic). - Improve worker pool example (@talex5 ocaml-multicore/eio#454). - Add more Conditions documentation (@talex5 ocaml-multicore/eio#436, reviewed by @haesbaert). This adds a discussion of conditions to the README and provides examples using them to handle signals. - Condition: fix the example in the docstring (@avsm ocaml-multicore/eio#468). Performance: - Add a network benchmark using an HTTP-like protocol (@talex5 ocaml-multicore/eio#478, reviewed by @avsm @patricoferris). - Add a benchmark for reading from `/dev/zero` (@talex5 ocaml-multicore/eio#439). Other changes: - Add CI for macOS (@talex5 ocaml-multicore/eio#452). - Add tests for `pread`, `pwrite` and `readdir` (@talex5 ocaml-multicore/eio#451). - eio_linux: split into multiple files (@talex5 ocaml-multicore/eio#465 ocaml-multicore/eio#466, reviewed by @avsm). - Update Dockerfile (@talex5 ocaml-multicore/eio#471). - Use dune.3.7.0 (@patricoferris ocaml-multicore/eio#457). - Mint exclusive IDs across domains (@TheLortex ocaml-multicore/eio#480, reported by @haesbaert, reviewed by @talex5). The tracing currently only works with a single domain anyway, but this will change when OCaml 5.1 is released.
This adds a new backend for POSIX systems. This is intended to be used as the fallback backend on most platforms, replacing libuv (see #434).
This isn't quite ready, but I thought some people might like to look it at early. Especially @patricoferris, as it may be relevant to his kqueue work.
The main commit uses the
poll
library, which has kqueue support, but a later commit replaces this with @haesbaert's newiomux
library. However, this might not work on macos.This is on top of #447. Use
make test_posix
to run the tests.TODO before merging:
pread[v]
,pwrite[v]
.readdir
support.O_NOFOLLOW
.openat
,mkdirat
, etc.Other things to work on after merging this initial support:
getaddrinfo
(see Getaddrinfo raises error on some backends #351).