Skip to content

Commit

Permalink
Merge pull request #734 from talex5/fix-signal-race
Browse files Browse the repository at this point in the history
eio_linux: add work-around for signals race
  • Loading branch information
talex5 authored May 26, 2024
2 parents c023b2e + ab12b0b commit 2c5eb61
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 3 deletions.
9 changes: 8 additions & 1 deletion lib_eio_linux/sched.ml
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,14 @@ let rec schedule ({run_q; sleep_q; mem_q; uring; _} as st) : [`Exit_scheduler] =
If [need_wakeup] is still [true], this is fine because we don't promise to do that.
If [need_wakeup = false], a wake-up event will arrive and wake us up soon. *)
Trace.suspend_domain Begin;
let result = Uring.wait ?timeout uring in
let result =
(* Hack: liburing automatically retries [io_uring_enter] if an
interrupt is received and no timeout is set. However, we need
to return to OCaml mode so any pending signal handlers can
run. See: https://github.com/ocaml-multicore/eio/issues/732 *)
let timeout = Option.value timeout ~default:1e9 in
Uring.wait ~timeout uring
in
Trace.suspend_domain End;
Atomic.set st.need_wakeup false;
Lf_queue.push run_q IO; (* Re-inject IO job in the run queue *)
Expand Down
18 changes: 16 additions & 2 deletions lib_eio_linux/tests/test.ml
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ let test_read_exact () =
let ( / ) = Eio.Path.( / ) in
let path = env#cwd / "test.data" in
let msg = "hello" in
Eio.Path.save path ("!" ^ msg) ~create:(`Exclusive 0o600);
Eio.Path.save path ("!" ^ msg) ~create:(`Or_truncate 0o600);
Switch.run @@ fun sw ->
let fd = Eio_linux.Low_level.openat2 ~sw
~access:`R
Expand Down Expand Up @@ -162,7 +162,7 @@ let test_statx () =
Eio_linux.run ~queue_depth:4 @@ fun env ->
let ( / ) = Eio.Path.( / ) in
let path = env#cwd / "test2.data" in
Eio.Path.with_open_out path ~create:(`Exclusive 0o600) @@ fun file ->
Eio.Path.with_open_out path ~create:(`Or_truncate 0o600) @@ fun file ->
Eio.Flow.copy_string "hello" file;
let buf = Uring.Statx.create () in
let test expected_len ~follow dir path =
Expand Down Expand Up @@ -198,6 +198,19 @@ let test_statx () =
);
()

(* Ensure that an OCaml signal handler will run, even if we're sleeping in liburing at the time.
The problem here is that [__sys_io_uring_enter2] doesn't return EINTR, because it did successfully
submit an item. This causes liburing to retry without giving our OCaml signal handler a chance to run.
Note: we can't run this test with a timeout because liburing does return in that case! *)
let test_signal_race () =
Eio_linux.run @@ fun _env ->
let cond = Eio.Condition.create () in
let handle _ = Eio.Condition.broadcast cond in
Sys.(set_signal sigalrm) (Signal_handle handle);
Fiber.both
(fun () -> Eio.Condition.await_no_mutex cond)
(fun () -> ignore (Unix.setitimer ITIMER_REAL { it_interval = 0.; it_value = 0.001 } : Unix.interval_timer_status))

let () =
let open Alcotest in
run "eio_linux" [
Expand All @@ -211,5 +224,6 @@ let () =
test_case "read_exact" `Quick test_read_exact;
test_case "expose_backend" `Quick test_expose_backend;
test_case "statx" `Quick test_statx;
test_case "signal_race" `Quick test_signal_race;
];
]

0 comments on commit 2c5eb61

Please sign in to comment.