Skip to content
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

Get Lwt and Eio to share the SIGCHLD handler #19

Merged
merged 1 commit into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions dune
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
(mdx
(packages lwt_eio)
(files README.md))
(files README.md)
(deps
(env_var "EIO_BACKEND")
(package lwt_eio)
(package eio_main)))
8 changes: 4 additions & 4 deletions dune-project
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
(lang dune 2.9)
(lang dune 3.8)
(name lwt_eio)
(formatting disabled)
(generate_opam_files true)
Expand All @@ -12,8 +12,8 @@
(synopsis "Run Lwt code within Eio")
(description "An Lwt engine that allows running Lwt within an Eio event loop.")
(depends
(eio (>= 0.10))
lwt
(eio (>= 0.11))
(lwt (>= 5.7.0))
(mdx (and (>= 1.10.0) :with-test))
(eio_main :with-test)))
(using mdx 0.1)
(using mdx 0.2)
31 changes: 31 additions & 0 deletions lib/lwt_eio.ml
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,33 @@ let fork_with_cancel ~sw fn =
Option.get !cancel
)

(* Lwt wants to set SIGCHLD to its own handler, but some Eio backends also install a handler for it.
Both Lwt and Eio need to be notified, as they may each have child processes to monitor.

There are two cases:

1. Eio installs its handler first, then Lwt tries to replace it while Eio is running.
We intercept that attempt and prevent the handler from changing.

2. Lwt installs its handler first (e.g. because someone ran a Lwt event loop for a bit before using Eio).
In that case, Eio will already have replaced Lwt's handler by the time we get called.

Either way, Eio ends up owning the installed handler. We also want things to continue working if the Eio
event loop finishes and then the application runs a plain Lwt loop. That's why we use [register_immediate],
rather than running an Eio fiber.

We also send an extra notification initially, in case we missed one during the hand-over. *)
let install_sigchld_handler = lazy (
if not Sys.win32 then (
Eio_unix.Process.install_sigchld_handler ();
let rec register () =
ignore (Eio.Condition.register_immediate Eio_unix.Process.sigchld register : Eio.Condition.request);
Lwt_unix.handle_signal Sys.sigchld
in
register ()
)
)

let make_engine ~sw ~clock = object
inherit Lwt_engine.abstract

Expand Down Expand Up @@ -78,6 +105,9 @@ let make_engine ~sw ~clock = object
Eio.Cancel.protect (fun () -> callback (); notify ())
)

method! forwards_signal signum =
signum = Sys.sigchld

method iter block =
if block then (
let p, r = Promise.create () in
Expand Down Expand Up @@ -110,6 +140,7 @@ let main ~clock user_promise =
Lwt_engine.set old_engine

let with_event_loop ~clock fn =
Lazy.force install_sigchld_handler;
let p, r = Lwt.wait () in
Switch.run @@ fun sw ->
Fiber.fork ~sw (fun () -> main ~clock p);
Expand Down
8 changes: 3 additions & 5 deletions lwt_eio.opam
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ homepage: "https://github.com/ocaml-multicore/lwt_eio"
doc: "https://ocaml-multicore.github.io/lwt_eio"
bug-reports: "https://github.com/ocaml-multicore/lwt_eio/issues"
depends: [
"dune" {>= "2.9"}
"eio" {>= "0.10"}
"lwt"
"dune" {>= "3.8"}
"eio" {>= "0.11"}
"lwt" {>= "5.7.0"}
"mdx" {>= "1.10.0" & with-test}
"eio_main" {with-test}
"odoc" {with-doc}
Expand All @@ -26,11 +26,9 @@ build: [
name
"-j"
jobs
"--promote-install-files=false"
"@install"
"@runtest" {with-test}
"@doc" {with-doc}
]
["dune" "install" "-p" name "--create-install-files" name]
]
dev-repo: "git+https://github.com/ocaml-multicore/lwt_eio.git"
5 changes: 4 additions & 1 deletion test/dune
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
(mdx
(packages lwt_eio))
(deps
(env_var "EIO_BACKEND")
(package lwt_eio)
(package eio_main)))
40 changes: 32 additions & 8 deletions test/test.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,7 @@ let run fn =

## Fairness

Lwt and Eio fibers don't block each other, although `Lwt_main.run` does call `Lwt.wakeup_paused` twice
during each iteration of the main loop, so the Lwt thread runs twice as often.
Lwt and Eio fibers don't block each other:

```ocaml
# run @@ fun () ->
Expand All @@ -42,19 +41,19 @@ during each iteration of the main loop, so the Lwt thread runs twice as often.
);;
+eio: i = 1
+ lwt: i = 1
+eio: i = 2
+ lwt: i = 2
+eio: i = 2
+ lwt: i = 3
+eio: i = 3
+ lwt: i = 4
+ lwt: i = 5
+eio: i = 4
+ lwt: i = 6
+ lwt: i = 7
+ lwt: i = 5
+eio: i = 5
+ lwt: i = 8
+ lwt: i = 6
+eio: i = 6
+ lwt: i = 7
+eio: i = 7
+ lwt: i = 8
+eio: i = 8
- : unit = ()
```
Expand All @@ -65,12 +64,37 @@ We can fork (as long we we're not using multiple domains):

```ocaml
# run @@ fun () ->
let output = Lwt_eio.run_lwt (fun () -> Lwt_process.(pread (shell "echo test"))) in
let output = Lwt_eio.run_lwt (fun () -> Lwt_process.(pread (shell "sleep 0.1; echo test"))) in
traceln "Subprocess produced %S" output;;
+Subprocess produced "test\n"
- : unit = ()
```

Lwt's SIGCHLD handling works:

```ocaml
# run @@ fun () ->
Lwt_eio.run_lwt (fun () ->
let p = Lwt_process.(open_process_none (shell "sleep 0.1; echo test")) in
let+ status = p#status in
assert (status = Unix.WEXITED 0)
);;
test
- : unit = ()
```

Eio processes work too:

```ocaml
# Eio_main.run @@ fun env ->
Lwt_eio.with_event_loop ~clock:env#clock @@ fun _ ->
Lwt_eio.run_lwt Lwt.pause;
let proc_mgr = Eio.Stdenv.process_mgr env in
Eio.Process.run proc_mgr ["echo"; "hello"];;
hello
- : unit = ()
```

## Signals

Installing a signal handler with `Lwt_unix.on_signal` causes `lwt_unix_send_notification` to be called when the signal occurs. This signals the main event thread by writing to a pipe or eventfd, which is processed by Eio in the usual way.
Expand Down