From d3cb04a67e41a0c3c55b0eb7659876a59622d5e9 Mon Sep 17 00:00:00 2001 From: bdodrem Date: Sat, 25 Jan 2025 14:10:11 +0100 Subject: [PATCH] eio_windows: fix blocking bug and pipe error * fix blocking issue on Windows : issue #793. Adding await_readable before reading fd * fix broken pipe exception : issue #792. Use Unix.read_bigarray instead of Unix_cstruct.read * replace eio_windows_cstruct_stubs.c by Unix functions. Since OCaml 5.2, Unix.read_bigarray and Unix.write_bigarray can be used. --- .github/workflows/main.yml | 4 +- dune-project | 2 +- eio.opam | 2 +- lib_eio_windows/dune | 2 +- lib_eio_windows/eio_windows_cstruct_stubs.c | 149 -------------------- lib_eio_windows/low_level.ml | 19 ++- lib_eio_windows/low_level.mli | 1 + 7 files changed, 21 insertions(+), 158 deletions(-) delete mode 100755 lib_eio_windows/eio_windows_cstruct_stubs.c diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index a920ba0be..466d27340 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -12,7 +12,7 @@ jobs: os: - macos-latest ocaml-compiler: - - 5.1.x + - 5.2.x local-packages: - eio eio_posix eio_main @@ -44,7 +44,7 @@ jobs: with: opam-pin: false opam-depext: false - ocaml-compiler: ocaml.5.1.0,ocaml-option-mingw + ocaml-compiler: ocaml.5.2.0,ocaml-option-mingw opam-repositories: | dra27: https://github.com/dra27/opam-repository.git#windows-5.0 normal: https://github.com/ocaml/opam-repository.git diff --git a/dune-project b/dune-project index e4dd40e56..51d1324e6 100644 --- a/dune-project +++ b/dune-project @@ -14,7 +14,7 @@ (description "An effect-based IO API for multicore OCaml with fibers.") (conflicts (seq (< 0.3))) (depends - (ocaml (>= 5.1.0)) + (ocaml (>= 5.2.0)) (bigstringaf (>= 0.9.0)) (cstruct (>= 6.0.1)) lwt-dllist diff --git a/eio.opam b/eio.opam index 7dafd13bb..4e1ffb63a 100644 --- a/eio.opam +++ b/eio.opam @@ -10,7 +10,7 @@ doc: "https://ocaml-multicore.github.io/eio/" bug-reports: "https://github.com/ocaml-multicore/eio/issues" depends: [ "dune" {>= "3.9"} - "ocaml" {>= "5.1.0"} + "ocaml" {>= "5.2.0"} "bigstringaf" {>= "0.9.0"} "cstruct" {>= "6.0.1"} "lwt-dllist" diff --git a/lib_eio_windows/dune b/lib_eio_windows/dune index 879134229..0fb6d8e0c 100644 --- a/lib_eio_windows/dune +++ b/lib_eio_windows/dune @@ -6,7 +6,7 @@ (foreign_stubs (language c) (include_dirs ../lib_eio/unix/include) - (names eio_windows_stubs eio_windows_cstruct_stubs)) + (names eio_windows_stubs)) (c_library_flags :standard -lbcrypt -lntdll) (libraries eio eio.unix eio.utils fmt)) diff --git a/lib_eio_windows/eio_windows_cstruct_stubs.c b/lib_eio_windows/eio_windows_cstruct_stubs.c deleted file mode 100755 index 863f41d7a..000000000 --- a/lib_eio_windows/eio_windows_cstruct_stubs.c +++ /dev/null @@ -1,149 +0,0 @@ -/* From mirage/ocaml-cstruct -Copyright (c) 2012 Anil Madhavapeddy -Copyright (c) 2012 Pierre Chambart -Copyright (c) Christiano F. Haesbaert -Copyright (c) Citrix Inc -Copyright (c) David Sheets -Copyright (c) Drup -Copyright (c) Hannes Mehnert -Copyright (c) Jeremy Yallop -Copyright (c) Mindy Preston -Copyright (c) Nicolas Ojeda Bar -Copyright (c) Richard Mortier -Copyright (c) Rudi Grinberg -Copyright (c) Thomas Gazagnaire -Copyright (c) Thomas Leonard -Copyright (c) Vincent Bernardoff -Copyright (c) pqwy - -Permission to use, copy, modify, and distribute this software for any -purpose with or without fee is hereby granted, provided that the above -copyright notice and this permission notice appear in all copies. - -THE SOFTWARE IS PROVIDED "AS IS" AND THE AUTHOR DISCLAIMS ALL WARRANTIES -WITH REGARD TO THIS SOFTWARE INCLUDING ALL IMPLIED WARRANTIES OF -MERCHANTABILITY AND FITNESS. IN NO EVENT SHALL THE AUTHOR BE LIABLE FOR -ANY SPECIAL, DIRECT, INDIRECT, OR CONSEQUENTIAL DAMAGES OR ANY DAMAGES -WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN -ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF -OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE. */ -#include -#include -#include -#include -#include -#include -#include -#include -#include - -#include -#include - -CAMLprim value eio_windows_cstruct_read(value val_fd, value val_c) -{ - CAMLparam2(val_fd, val_c); - CAMLlocal3(val_buf, val_ofs, val_len); - uint8_t *buf; - size_t len; - ssize_t n = 0; - int win32err = 0; - SOCKET s; - HANDLE h; - DWORD numread; - int ok; - - val_buf = Field(val_c, 0); - val_ofs = Field(val_c, 1); - val_len = Field(val_c, 2); - - buf = (uint8_t *)Caml_ba_data_val(val_buf) + Long_val(val_ofs); - len = (size_t)Long_val(val_len); - - switch (Descr_kind_val(val_fd)) - { - case KIND_SOCKET: - s = Socket_val(val_fd); - - caml_release_runtime_system(); - n = recv(s, buf, len, 0); - win32err = WSAGetLastError(); - caml_acquire_runtime_system(); - - if (n == SOCKET_ERROR) - { - win32_maperr(win32err); - uerror("stub_cstruct_read", Nothing); - } - break; - case KIND_HANDLE: - h = Handle_val(val_fd); - caml_release_runtime_system(); - ok = ReadFile(h, buf, len, &numread, NULL); - win32err = GetLastError(); - n = numread; - caml_acquire_runtime_system(); - - if (!ok) - { - win32_maperr(win32err); - uerror("stub_cstruct_read", Nothing); - } - break; - default: - caml_failwith("unknown Descr_kind_val"); - } - - CAMLreturn(Val_int(n)); -} - -CAMLprim value eio_windows_cstruct_write(value val_fd, value val_c) -{ - CAMLparam2(val_fd, val_c); - CAMLlocal3(val_buf, val_ofs, val_len); - val_buf = Field(val_c, 0); - val_ofs = Field(val_c, 1); - val_len = Field(val_c, 2); - void *buf = (char *)Caml_ba_data_val(val_buf) + Long_val(val_ofs); - size_t len = Long_val(val_len); - ssize_t n = 0; - - int win32err = 0; - switch (Descr_kind_val(val_fd)) - { - case KIND_SOCKET: - SOCKET s = Socket_val(val_fd); - - caml_release_runtime_system(); - n = send(s, buf, len, 0); - win32err = WSAGetLastError(); - caml_acquire_runtime_system(); - - if (n == SOCKET_ERROR) - { - win32_maperr(win32err); - unix_error(errno, "stub_cstruct_write", Nothing); - } - break; - case KIND_HANDLE: - HANDLE h = Handle_val(val_fd); - DWORD numwritten; - caml_release_runtime_system(); - int ok = WriteFile(h, buf, len, &numwritten, NULL); - win32err = GetLastError(); - - n = numwritten; - caml_acquire_runtime_system(); - - if (!ok) - { - win32_maperr(win32err); - uerror("stub_cstruct_write", Nothing); - } - break; - default: - caml_failwith("unknown Descr_kind_val"); - } - - CAMLreturn(Val_int(n)); -} \ No newline at end of file diff --git a/lib_eio_windows/low_level.ml b/lib_eio_windows/low_level.ml index 85f25cd26..8a9160e52 100755 --- a/lib_eio_windows/low_level.ml +++ b/lib_eio_windows/low_level.ml @@ -39,17 +39,25 @@ let rec do_nonblocking ty fn fd = do_nonblocking ty fn fd let read fd buf start len = + await_readable fd; Fd.use_exn "read" fd @@ fun fd -> do_nonblocking Read (fun fd -> Unix.read fd buf start len) fd -let read_cstruct fd buf = +let read_cstruct fd (buf:Cstruct.t) = + await_readable fd; Fd.use_exn "read_cstruct" fd @@ fun fd -> - do_nonblocking Read (fun fd -> Unix_cstruct.read fd buf) fd + do_nonblocking Read (fun fd -> Unix.read_bigarray fd buf.buffer buf.off buf.len) fd let write fd buf start len = + await_writable fd; Fd.use_exn "write" fd @@ fun fd -> do_nonblocking Write (fun fd -> Unix.write fd buf start len) fd +let write_cstruct fd (buf:Cstruct.t) = + await_writable fd; + Fd.use_exn "write_cstruct" fd @@ fun fd -> + do_nonblocking Write (fun fd -> Unix.write_bigarray fd buf.buffer buf.off buf.len) fd + let sleep_until time = Sched.enter @@ fun t k -> Sched.await_timeout t k time @@ -148,8 +156,11 @@ let readv fd bufs = do_nonblocking Read (fun fd -> eio_readv fd bufs) fd let writev fd bufs = - Fd.use_exn "writev" fd @@ fun fd -> - do_nonblocking Write (fun fd -> Unix_cstruct.writev fd bufs) fd + let rec loop buf = if Cstruct.length buf > 0 then begin + let n = write_cstruct fd buf in + loop @@ Cstruct.shift buf n + end in + List.iter loop bufs let preadv ~file_offset fd bufs = Fd.use_exn "preadv" fd @@ fun fd -> diff --git a/lib_eio_windows/low_level.mli b/lib_eio_windows/low_level.mli index e2ec400bd..08315c15e 100755 --- a/lib_eio_windows/low_level.mli +++ b/lib_eio_windows/low_level.mli @@ -22,6 +22,7 @@ val sleep_until : Mtime.t -> unit val read : fd -> bytes -> int -> int -> int val read_cstruct : fd -> Cstruct.t -> int val write : fd -> bytes -> int -> int -> int +val write_cstruct : fd -> Cstruct.t -> int val socket : sw:Switch.t -> Unix.socket_domain -> Unix.socket_type -> int -> fd val connect : fd -> Unix.sockaddr -> unit