-
Notifications
You must be signed in to change notification settings - Fork 399
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
fix(sendfile): reuse fd before fallback #8234
Conversation
otherlibs/stdune/src/io.ml
Outdated
@@ -179,6 +179,7 @@ module Copyfile = struct | |||
let sendfile_with_fallback ?chmod ~src ~dst () = | |||
try sendfile ?chmod ~src ~dst () | |||
with Unix.Unix_error (EINVAL, "sendfile", _) -> | |||
Unix.unlink dst; |
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 this the correct place to do this, or can it be done inside sendfile
when it aborts?
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 there is some similar cleanup for src there too.
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 thought as well, but this happens in the prepare phase, not when sendfile
itself fails.
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.
Thanks. This is indeed simpler.
Fixes ocaml#8210 If the `sendfile` branch failed, it leaves an empty target file, so retrying will fail. Instead we reuse the fd directly. Signed-off-by: Etienne Millon <me@emillon.org>
Fixes ocaml#8210 If the `sendfile` branch failed, it leaves an empty target file, so retrying will fail. Instead we reuse the fd directly. Signed-off-by: Etienne Millon <me@emillon.org>
Fixes ocaml#8210 If the `sendfile` branch failed, it leaves an empty target file, so retrying will fail. Instead we reuse the fd directly. Signed-off-by: Etienne Millon <me@emillon.org>
* test: repro for #8210 (EACCESS if sendfile fails) What's happening here is: - input and output files are open - sendfile is attempted and fails - both files are closed - input file is opened again - output file is opened again (but already exists) This throws an error if the output file is not writable. Signed-off-by: Etienne Millon <me@emillon.org> * refactor: use Exn.protect instead of protectx (#8235) This adds a layer of rebinding that obscures what is going on. Signed-off-by: Etienne Millon <me@emillon.org> * fix(sendfile): unlink dst before fallback (#8234) Fixes #8210 If the `sendfile` branch failed, it leaves an empty target file, so retrying will fail. Instead we reuse the fd directly. Signed-off-by: Etienne Millon <me@emillon.org> --------- Signed-off-by: Etienne Millon <me@emillon.org>
CHANGES: - Disable background digests on Windows. This prevents an issue where unremovable files would make dune crash when the shared cache is enabled. (ocaml/dune#8243, fixes ocaml/dune#8228, @emillon) - Fix permission errors when `sendfile` is not available (ocaml/dune#8234, fixes ocaml/dune#8120, @emillon)
Fixes ocaml#8210 If the `sendfile` branch failed, it leaves an empty target file, so retrying will fail. Instead we reuse the fd directly. Signed-off-by: Etienne Millon <me@emillon.org>
CHANGES: - Disable background digests on Windows. This prevents an issue where unremovable files would make dune crash when the shared cache is enabled. (ocaml/dune#8243, fixes ocaml/dune#8228, @emillon) - Fix permission errors when `sendfile` is not available (ocaml/dune#8234, fixes ocaml/dune#8120, @emillon)
Fixes #8210
If the
sendfile
branch failed, it left an empty target file.This file needs to be removed, otherwise the next
open
call can fail if the file is not writeable.