CP-32622: Remove 'select' from stdext #80
Conversation
edwintorok
changed the title
edwintorok
force-pushed
the
private/edvint/noselect
branch
from
a0e18d0 to
52b4555
Compare
| let proxy (a: Unix.file_descr) (b: Unix.file_descr) = | ||
| let size = 64 * 1024 in | ||
| (* [a'] is read from [a] and will be written to [b] *) | ||
| (* [b'] is read from [b] and will be written to [a] *) | ||
| let a' = CBuf.empty size and b' = CBuf.empty size in | ||
| Unix.set_nonblock a; | ||
| Unix.set_nonblock b; | ||
| with_polly @@ fun polly -> | ||
|
|
||
| try | ||
| while true do |
There was a problem hiding this comment.
could this be a recursive function with any as a parameter which is used for termination?
xapi-stdext-unix-opam.template
Outdated
| @@ -0,0 +1,2 @@ | |||
| depexts: ["linux-headers"] {os-distribution = "alpine"} | |||
| available: [ os = "macos" | os = "linux" ] | |||
There was a problem hiding this comment.
epoll is not available on macOS; not sure if this is relevant here.
There was a problem hiding this comment.
We could use iomux if needed, it supports more OSes. If we want to make xe available on more platforms then we should either remove its dependency on stdext, or avoid hard dependencies on epoll.
For now I've updated the template to drop macos.
There was a problem hiding this comment.
Although contrary to its claim in the package description 'epoll' support isn't actually implement in iomux yet, it is just poll. Although for most purposes that is enough, epoll is useful if you want to watch a large number of FDs in one go (e.g. XAPI webserver)
edwintorok
force-pushed
the
private/edvint/noselect
branch
from
cf10a16 to
95197c9
Compare
|
I hadn't noticed that there is already #71 which also has one unit test. That is very error prone, so I introduced a Also fixed up the unit test so that it prints the expected and actually measured time instead of just printing 'false', which is very useful for debugging what went wrong. |
edwintorok
force-pushed
the
private/edvint/noselect
branch
from
95197c9 to
e0a1930
Compare
edwintorok
force-pushed
the
private/edvint/noselect
branch
3 times, most recently
from
b7902c6 to
ff5b1b6
Compare
'make format' with ocamlformat 0.22.4 No functional change. Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Steven Woods <[email protected]> Signed-off-by: Edwin Török <[email protected]>
`Unix.write` would internally loop until the desired number of bytes are sent,
or `EAGAIN`/`EWOULDBLOCK`/another error is reached.
It cannot check for timeouts because it is not aware that we'd want one.
For pipes and sockets in non-blocking mode this wouldn't be a problem, but this function
is often called with block devices too.
However according to `pselect(3p)` it is a no-op on regular files:
"File descriptors associated with regular files shall always select true for ready to read, ready to write, and error conditions."
And the behaviour on block devices is left unspecified by POSIX, and `select(2)` on Linux doesn't document the behaviour either:
"The pselect() and select() functions shall support regular files, terminal and pseudo‐terminal devices, STREAMS‐based files, FIFOs, pipes, and sockets. The behavior of pselect() and select() on file descriptors that refer to other types of file is unspecified"
Although timeouts for a completely stuck block device cannot be implemented, we can still implement timeouts for a *slow* block device.
Use `Unix.single_{write,read}` instead which gives full control of the iteration to the caller.
The only way to interrupt stuck IO on a block device or regular file would be to use a separate process and `SIGKILL` it after a timeout (this is what `block_device_io` in XAPI does).
These approaches do not work:
* `alarm` or `setitimer` would only interrupt one thread in a multi-threaded program.
* `pthread_kill` can be used to send a signal to a particular thread, but that requires `EINTR` behaviour on syscalls to be enabled
* XAPI expects `SA_RESTART` semantics from syscalls, and would fail an assertion if it ever sees `EINTR` in some paths,
so although the syscall *would* get interrupted, it'd also immediately resume without giving the caller a chance to check for timeouts
* even if it'd work there are no bindings to `pthread_kill` in OCaml
Signed-off-by: Edwin Török <[email protected]>
'select' has a hardcoded limit of 1024 file descriptors. Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Edwin Török <[email protected]>
It is too easy to misuse Unixext.time_limited_read because that one takesan absolute timestamp as parameter, not a relative one. Introduce a new function that takes a relative time as parameter, and doesn't loop. Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Edwin Török <[email protected]>
Signed-off-by: Edwin Török <[email protected]>
edwintorok
force-pushed
the
private/edvint/noselect
branch
from
ff5b1b6 to
4456546
Compare
I'll try to find out some time to add a timer module I've been developing lately to module in the date / time package that can be used for timed functions: module Timer : sig
(** This module is useful for knowing that a set amount of time has passed
after some event happened. For example, to know when pasta is cooked al
dente. *)
type t
type remaining = Spare of Mtime.Span.t | Excess of Mtime.Span.t
val start : timeout:Mtime.Span.t -> t
(** [start ~timeout] starts a timer that expires after [timeout] has passed.
The wait is done in monotonic time, not in POSIX time. *)
val timeout : t -> Mtime.Span.t
(** [timeout timer] returns the amount of time after which the timer expires,
from the moment it was started. *)
val expired : t -> bool
(** [expired timer] returns whether [timer] has reached the timeout it was
set to. *)
val elapsed : t -> Mtime.Span.t
(** [elapsed timer] returns the amount of time elapsed since [timer] was
started. *)
val remaining : t -> remaining
(** [remaining timer] returns the amount of spare time is left until it
expires, or the amount of excess time since it expired. *)
val deadline_of : t -> float
(** [deadline_of timer] returns the posix timestamp when the timer expires.
This is an approximation as the timer doesn't take leap seconds into
account when waiting. The use of this function is discouraged and it's
only provided for backwards-compatible reasons. *)
endThe implementation can be found in this branch: psafont/xen-api@c9ebc65 |
|
We'd also need to convert that into whatever form the underlying POSIX API expects, and they may use different clocks. Shouldn't we just merge stdext into XAPI itself? it looks like we might want to refactor some of the APIs, and having them in separate repos like this makes that more difficult (I'd prefer if XAPI used mtime too, but we cannot do that easily without breaking changes, whereas in a monorepo it'd be a lot simpler). |
|
Continued in xapi-project/xen-api#5402 |
Draft, because I need to add some unit tests here.
I'd also like to add a separate function that works only on sockets which is much simpler and uses just 'setsockopt_float'.