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

eio_linux: add work-around for signals race #734

Merged
merged 1 commit into from
May 26, 2024
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
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;
];
]
Loading