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

Lwt_unix.fork causes trouble #15

Closed
talex5 opened this issue Feb 2, 2023 · 4 comments · Fixed by #19
Closed

Lwt_unix.fork causes trouble #15

talex5 opened this issue Feb 2, 2023 · 4 comments · Fixed by #19

Comments

@talex5
Copy link
Collaborator

talex5 commented Feb 2, 2023

The tests run Lwt_process.(pread (shell "echo test"), but this isn't reliable.

Simpler test case:

open Eio.Std

let () =
  Eio_main.run @@ fun env ->
  Lwt_eio.with_event_loop ~clock:env#clock @@ fun _ ->
  for _ = 1 to 100 do
    let output = Lwt_eio.run_lwt (fun () ->
        match Lwt_unix.fork () with
        | 0 -> Unix._exit 1
        | _child -> Lwt.return "ok"
      ) in
    traceln "got: %s" output
  done

Fails with:

+got: ok
+got: ok
fish: “dune exec -- ./examples/simple.…” terminated by signal SIGSEGV (Address boundary error)

(the number of "ok"s varies)

@talex5
Copy link
Collaborator Author

talex5 commented Feb 3, 2023

This is a bug in Lwt. It needs to use sigact.sa_flags = SA_ONSTACK.

@talex5
Copy link
Collaborator Author

talex5 commented May 18, 2023

A work-around is to do this before running the Eio mainloop:

let () =
  (* Let Lwt install its buggy signal handler now. Then we'll put it back. *)
  Lwt_main.run (Lwt.pause ());
  Sys.(set_signal sigchld) Signal_default

With this, you can't spawn processes using Lwt, but you can using Eio, and there's no memory corruption.

@talex5
Copy link
Collaborator Author

talex5 commented Jun 5, 2023

Probably needs an extension to Lwt's engine type: ocsigen/lwt#991

@smondet
Copy link

smondet commented Jun 29, 2023

Just mentioning that the work-around above also seems to have worked to fix my code that was apparently hanging on Lwt_preemptive.detach (I'm experimenting with Caqti_lwt's sqlite backend, which uses system threads, I think that was the problem).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants