CA-387588: Unixext.really_read: restart on EINTR #86
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
To reproduce a forkexecd unit test failure I've run it under 'rr'. This caused a different failure: EINTR from read. Unixext already has code to defend against EINTR on write, but not on read: add missing loop. Signed-off-by: Edwin Török <[email protected]>
psafont
approved these changes
Jan 17, 2024
There was a problem hiding this comment.
Could you add a description to the changelog?
Similar to this: CHANGES.md
lindig
approved these changes
Jan 17, 2024
Thanks, looks like you already did that for me in the other PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
To reproduce a forkexecd unit test failure I've run it under rr. This caused a different failure: EINTR from read.
Unixext already has code to defend against EINTR on write, but not on read: add missing loop.
It is unclear whether this is a bug in
rror not, but according to thesignal(7)manpage some syscalls may returnEINTReven whenRESTARTSYShandling is set for the signal, butreadisn't one of those syscalls:https://man7.org/linux/man-pages/man7/signal.7.html see "Interruption of system calls and library functions by signal handlers"
And although RESTARTSYS seems to be the default handling for signals, if you establish a custom handler for any signal in OCaml I think you may always risk getting
EINTR, because thesigactioncall in OCaml doesn't pass theSA_RESTARTSYSflag.(which probably means we have bugs elsewhere like this in XAPI, I know of at least one place that would assert/raise exception on EINTR on the basis that it should never be received, I think it is one of the
selectcalls in fact...)