From dddc0735eb227bbb4ce4f692e238ed559ae8ce7e Mon Sep 17 00:00:00 2001 From: Thomas Leonard Date: Wed, 19 Jan 2022 10:58:49 +0000 Subject: [PATCH] Add `Eio_unix.FD` This allows creating FDs with Eio and then using them in legacy code. --- dune-project | 3 ++- eio_luv.opam | 3 ++- lib_eio/eio.mli | 1 + lib_eio/net.ml | 3 ++- lib_eio/unix/eio_unix.ml | 7 +++++++ lib_eio/unix/eio_unix.mli | 18 +++++++++++++++++- lib_eio_linux/eio_linux.ml | 14 +++++++++++++- lib_eio_linux/eio_linux.mli | 5 +++-- lib_eio_linux/tests/test.ml | 6 +++--- lib_eio_luv/dune | 2 +- lib_eio_luv/eio_luv.ml | 37 +++++++++++++++++++++++++++++++++++-- tests/test_fs.md | 32 ++++++++++++++++++++++++++++++++ tests/test_network.md | 26 ++++++++++++++++++++++++++ 13 files changed, 144 insertions(+), 13 deletions(-) diff --git a/dune-project b/dune-project index e05dfc42a..f7632fe3d 100644 --- a/dune-project +++ b/dune-project @@ -42,7 +42,8 @@ base-domains (ctf (= :version)) (eio (= :version)) - (luv (>= 0.5.8)) + (luv (>= 0.5.11)) + (luv_unix (>= 0.5.0)) (mdx (and (>= 1.10.0) :with-test)) (logs (>= 0.7.0)) (fmt (>= 0.8.9)))) diff --git a/eio_luv.opam b/eio_luv.opam index 15a1eb84b..f701543a7 100644 --- a/eio_luv.opam +++ b/eio_luv.opam @@ -12,7 +12,8 @@ depends: [ "base-domains" "ctf" {= version} "eio" {= version} - "luv" {>= "0.5.8"} + "luv" {>= "0.5.11"} + "luv_unix" {>= "0.5.0"} "mdx" {>= "1.10.0" & with-test} "logs" {>= "0.7.0"} "fmt" {>= "0.8.9"} diff --git a/lib_eio/eio.mli b/lib_eio/eio.mli index 926a1b4e5..adf4229b5 100644 --- a/lib_eio/eio.mli +++ b/lib_eio/eio.mli @@ -720,6 +720,7 @@ module Net : sig end class virtual listening_socket : object + inherit Generic.t method virtual close : unit method virtual accept : sw:Switch.t -> * Sockaddr.t end diff --git a/lib_eio/net.ml b/lib_eio/net.ml index d4bff486d..40152d45f 100644 --- a/lib_eio/net.ml +++ b/lib_eio/net.ml @@ -126,7 +126,8 @@ module Sockaddr = struct Format.fprintf f "tcp:%a:%d" Ipaddr.pp_for_uri addr port end -class virtual listening_socket = object +class virtual listening_socket = object (_ : #Generic.t) + method probe _ = None method virtual close : unit method virtual accept : sw:Switch.t -> * Sockaddr.t end diff --git a/lib_eio/unix/eio_unix.ml b/lib_eio/unix/eio_unix.ml index c5c2b477e..a19496b91 100644 --- a/lib_eio/unix/eio_unix.ml +++ b/lib_eio/unix/eio_unix.ml @@ -1,5 +1,7 @@ open Eio.Private.Effect +type _ Eio.Generic.ty += Unix_file_descr : [`Peek | `Take] -> Unix.file_descr Eio.Generic.ty + module Effects = struct type _ eff += | Await_readable : Unix.file_descr -> unit eff @@ -9,6 +11,11 @@ end let await_readable fd = perform (Effects.Await_readable fd) let await_writable fd = perform (Effects.Await_writable fd) +module FD = struct + let peek x = Eio.Generic.probe x (Unix_file_descr `Peek) + let take x = Eio.Generic.probe x (Unix_file_descr `Take) +end + module Ipaddr = struct let to_unix : _ Eio.Net.Ipaddr.t -> Unix.inet_addr = Obj.magic let of_unix : Unix.inet_addr -> _ Eio.Net.Ipaddr.t = Obj.magic diff --git a/lib_eio/unix/eio_unix.mli b/lib_eio/unix/eio_unix.mli index e0f786f92..87df16de7 100644 --- a/lib_eio/unix/eio_unix.mli +++ b/lib_eio/unix/eio_unix.mli @@ -1,4 +1,8 @@ -(** Extension of {!Eio} for integration with OCaml's [Unix] module. *) +(** Extension of {!Eio} for integration with OCaml's [Unix] module. + + Note that OCaml's [Unix] module is not safe, and therefore care must be taken when using these functions. + For example, it is possible to leak file descriptors this way, or to use them after they've been closed, + allowing one module to corrupt a file belonging to an unrelated module. *) val await_readable : Unix.file_descr -> unit (** [await_readable fd] blocks until [fd] is readable (or has an error). *) @@ -6,6 +10,18 @@ val await_readable : Unix.file_descr -> unit val await_writable : Unix.file_descr -> unit (** [await_writable fd] blocks until [fd] is writable (or has an error). *) +type _ Eio.Generic.ty += Unix_file_descr : [`Peek | `Take] -> Unix.file_descr Eio.Generic.ty + +module FD : sig + val peek : #Eio.Generic.t -> Unix.file_descr option + (** [peek x] is the Unix file descriptor underlying [x], if any. + The caller must ensure that they do not continue to use the result after [x] is closed. *) + + val take : #Eio.Generic.t -> Unix.file_descr option + (** [take x] is like [peek], but also marks [x] as closed on success (without actually closing the FD). + [x] can no longer be used after this, and the caller is responsible for closing the FD. *) +end + module Ipaddr : sig val to_unix : [< `V4 | `V6] Eio.Net.Ipaddr.t -> Unix.inet_addr val of_unix : Unix.inet_addr -> Eio.Net.Ipaddr.v4v6 diff --git a/lib_eio_linux/eio_linux.ml b/lib_eio_linux/eio_linux.ml index 42679f3eb..3190277e0 100644 --- a/lib_eio_linux/eio_linux.ml +++ b/lib_eio_linux/eio_linux.ml @@ -79,7 +79,14 @@ module FD = struct | (_ : int) -> true | exception Unix.Unix_error(Unix.ESPIPE, "lseek", "") -> false - let to_unix = get "to_unix" + let to_unix op t = + let fd = get "to_unix" t in + match op with + | `Peek -> fd + | `Take -> + t.fd <- `Closed; + Eio.Hook.remove t.release_hook; + fd let of_unix_no_hook ~seekable ~close_unix fd = { seekable; close_unix; fd = `Open fd; release_hook = Eio.Hook.null } @@ -762,6 +769,7 @@ module Objects = struct method probe : type a. a Eio.Generic.ty -> a option = function | FD -> Some fd + | Eio_unix.Unix_file_descr op -> Some (FD.to_unix op fd) | _ -> None method read_into buf = @@ -798,6 +806,10 @@ module Objects = struct let listening_socket fd = object inherit Eio.Net.listening_socket + method! probe : type a. a Eio.Generic.ty -> a option = function + | Eio_unix.Unix_file_descr op -> Some (FD.to_unix op fd) + | _ -> None + method close = FD.close fd method accept ~sw = diff --git a/lib_eio_linux/eio_linux.mli b/lib_eio_linux/eio_linux.mli index 83312f774..910200533 100644 --- a/lib_eio_linux/eio_linux.mli +++ b/lib_eio_linux/eio_linux.mli @@ -39,9 +39,10 @@ module FD : sig @param seekable If true, we pass [-1] as the file offset, to use the current offset. If false, pass [0] as the file offset, which is needed for sockets. *) - val to_unix : t -> Unix.file_descr - (** [to_unix t] returns the wrapped descriptor. + val to_unix : [< `Peek | `Take] -> t -> Unix.file_descr + (** [to_unix op t] returns the wrapped descriptor. This allows unsafe access to the FD. + If [op] is [`Take] then [t] is marked as closed (but the underlying FD is not actually closed). @raise Invalid_arg if [t] is closed. *) end diff --git a/lib_eio_linux/tests/test.ml b/lib_eio_linux/tests/test.ml index 238b41b4f..a91a086f2 100644 --- a/lib_eio_linux/tests/test.ml +++ b/lib_eio_linux/tests/test.ml @@ -10,7 +10,7 @@ let read_one_byte ~sw r = let r = Option.get (Eio_linux.Objects.get_fd_opt r) in Eio_linux.await_readable r; let b = Bytes.create 1 in - let got = Unix.read (Eio_linux.FD.to_unix r) b 0 1 in + let got = Unix.read (Eio_linux.FD.to_unix `Peek r) b 0 1 in assert (got = 1); Bytes.to_string b ) @@ -23,7 +23,7 @@ let test_poll_add () = Fibre.yield (); let w = Option.get (Eio_linux.Objects.get_fd_opt w) in Eio_linux.await_writable w; - let sent = Unix.write (Eio_linux.FD.to_unix w) (Bytes.of_string "!") 0 1 in + let sent = Unix.write (Eio_linux.FD.to_unix `Peek w) (Bytes.of_string "!") 0 1 in assert (sent = 1); let result = Promise.await thread in Alcotest.(check string) "Received data" "!" result @@ -35,7 +35,7 @@ let test_poll_add_busy () = let a = read_one_byte ~sw r in let b = read_one_byte ~sw r in Fibre.yield (); - let w = Option.get (Eio_linux.Objects.get_fd_opt w) |> Eio_linux.FD.to_unix in + let w = Option.get (Eio_linux.Objects.get_fd_opt w) |> Eio_linux.FD.to_unix `Peek in let sent = Unix.write w (Bytes.of_string "!!") 0 2 in assert (sent = 2); let a = Promise.await a in diff --git a/lib_eio_luv/dune b/lib_eio_luv/dune index 6d6304aef..92ab1f26c 100644 --- a/lib_eio_luv/dune +++ b/lib_eio_luv/dune @@ -1,4 +1,4 @@ (library (name eio_luv) (public_name eio_luv) - (libraries eio.unix luv eio.utils logs fmt ctf)) + (libraries eio.unix luv luv_unix eio.utils logs fmt ctf)) diff --git a/lib_eio_luv/eio_luv.ml b/lib_eio_luv/eio_luv.ml index 6c25e6801..f5a2612f5 100644 --- a/lib_eio_luv/eio_luv.ml +++ b/lib_eio_luv/eio_luv.ml @@ -164,6 +164,18 @@ module Handle = struct let t = of_luv_no_hook fd in t.release_hook <- Switch.on_release_cancellable sw (fun () -> ensure_closed t); t + + let to_unix_opt op (t:_ t) = + match Luv.Handle.fileno (to_luv t) with + | Error _ -> None + | Ok os_fd -> + let fd = Luv_unix.Os_fd.Fd.to_unix os_fd in + match op with + | `Peek -> Some fd + | `Take -> + t.fd <- `Closed; + Eio.Hook.remove t.release_hook; + Some fd end module File = struct @@ -226,6 +238,16 @@ module File = struct let mkdir ~mode path = let request = Luv.File.Request.make () in await_with_cancel ~request (fun loop -> Luv.File.mkdir ~loop ~request ~mode path) + + let to_unix op t = + let os_fd = Luv.File.get_osfhandle (get "to_unix" t) |> or_raise in + let fd = Luv_unix.Os_fd.Fd.to_unix os_fd in + match op with + | `Peek -> fd + | `Take -> + t.fd <- `Closed; + Eio.Hook.remove t.release_hook; + fd end module Random = struct @@ -273,6 +295,8 @@ module Stream = struct match Luv.Buffer.drop bufs n |> skip_empty with | [] -> () | bufs -> write t bufs + + let to_unix_opt = Handle.to_unix_opt end module Poll = struct @@ -335,6 +359,7 @@ module Objects = struct method probe : type a. a Eio.Generic.ty -> a option = function | FD -> Some fd + | Eio_unix.Unix_file_descr op -> Some (File.to_unix op fd) | _ -> None method read_into buf = @@ -360,7 +385,11 @@ module Objects = struct let sink fd = (flow fd :> sink) let socket sock = object - inherit Eio.Flow.two_way + inherit Eio.Flow.two_way as super + + method! probe : type a. a Eio.Generic.ty -> a option = function + | Eio_unix.Unix_file_descr op -> Stream.to_unix_opt op sock + | x -> super#probe x method read_into buf = let buf = Cstruct.to_bigarray buf in @@ -390,7 +419,11 @@ module Objects = struct end class virtual ['a] listening_socket ~backlog sock = object (self) - inherit Eio.Net.listening_socket + inherit Eio.Net.listening_socket as super + + method! probe : type a. a Eio.Generic.ty -> a option = function + | Eio_unix.Unix_file_descr op -> Stream.to_unix_opt op sock + | x -> super#probe x val ready = Eio.Semaphore.make 0 diff --git a/tests/test_fs.md b/tests/test_fs.md index 842492191..5207a40ac 100644 --- a/tests/test_fs.md +++ b/tests/test_fs.md @@ -251,3 +251,35 @@ Exception: Eio.Dir.Permission_denied ("/dev/null", _) +Line: three - : unit = () ``` + +# Unix interop + +We can get the Unix FD from the flow and use it directly: + +```ocaml +# run @@ fun env -> + let fs = Eio.Stdenv.fs env in + Eio.Dir.with_open_in fs Filename.null (fun flow -> + match Eio_unix.FD.peek flow with + | None -> failwith "No Unix file descriptor!" + | Some fd -> + let got = Unix.read fd (Bytes.create 10) 0 10 in + traceln "Read %d bytes from null device" got + );; ++Read 0 bytes from null device +- : unit = () +``` + +We can also remove it from the flow completely and take ownership of it. +In that case, `with_open_in` will no longer close it on exit: + +```ocaml +# run @@ fun env -> + let fs = Eio.Stdenv.fs env in + let fd = Eio.Dir.with_open_in fs Filename.null (fun flow -> Option.get (Eio_unix.FD.take flow)) in + let got = Unix.read fd (Bytes.create 10) 0 10 in + traceln "Read %d bytes from null device" got; + Unix.close fd;; ++Read 0 bytes from null device +- : unit = () +``` diff --git a/tests/test_network.md b/tests/test_network.md index 4fb6ed306..fdf27b9a3 100644 --- a/tests/test_network.md +++ b/tests/test_network.md @@ -147,8 +147,34 @@ Calling accept when the switch is already off: ~on_error:raise;; Exception: Failure "Simulated error". ``` + # Unix interop +Extracting file descriptors from Eio objects: + +```ocaml +# run @@ fun ~net sw -> + let server = Eio.Net.listen net ~sw ~reuse_addr:true ~backlog:5 addr in + traceln "Listening socket has Unix FD: %b" (Eio_unix.FD.peek server <> None); + let have_client, have_server = + Fibre.pair + (fun () -> + let flow = Eio.Net.connect ~sw net addr in + (Eio_unix.FD.peek flow <> None) + ) + (fun () -> + let flow, _addr = Eio.Net.accept ~sw server in + (Eio_unix.FD.peek flow <> None) + ) + in + traceln "Client-side socket has Unix FD: %b" have_client; + traceln "Server-side socket has Unix FD: %b" have_server;; ++Listening socket has Unix FD: true ++Client-side socket has Unix FD: true ++Server-side socket has Unix FD: true +- : unit = () +``` + Check we can convert Eio IP addresses to Unix: ```ocaml