-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
fix fork()
usage in ocamltest
#11111
Conversation
Note: I wonder if there is a deeper problem here.
Maybe we can recommend to always use (Note: even with 4.x, there were reasons to recommend this, for example for Eventlog users, as |
It's not obvious to me why the child process uses the OCaml runtime. If we were to implement spawning using |
The In the medium term, maybe we could reconsider the idea of maintaining yet another implementation of |
Well, I find this questionable, even in OCaml 4. For example, there's a risk of double-flushing of OCaml channels (once in the parent, once in the child). resulting in duplicate output. |
Good point. I did wonder about flushing before the On the other hand, I think we do have OCaml programs that |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, let's merge this as a bug fix and think about alternate implementations later. Can you please fix the Changes entry then squash-and-merge?
I really hope this pattern ( |
Thanks! I updated and rebased.
It fails on my machine on the first time that we try to release the runtime lock / enter a blocking section. (I'm slightly nervous at the idea that our options to fix the issue in a backward-compatible ways will be very limited once 5.0 and 4.14 are release without a coherent support for forking, but oh well, let's assume that nobody does that indeed.) |
The error-reporting code in ocamltest's
run_unix.c
is incorrect with respect to the new Multicore runtime, because it tries to use the OCaml runtime (to write to an OCaml channel) without callingcaml_atfork_hook
first.reproduction information
Add
at the entry of run_command_child in ocamltest/run_unix.c, then run
Observed output contains:
On my system, the core dump has the following backtrace:
The problem comes from trying to use the OCaml runtime from a forked
child, without calling the new Multicore-OCaml function
caml_atfork_hook
from domain.h. At this point the forked child hasits domain lock in an incoherent state, and calling
caml_write_fd
fails. (Compare the ceremony around fork() in ocamltest/run_unix.c and
in otherlibs/unix/unix_fork.c)
Note: the backtrace also shows that C files in ocamltest were not
compiled with debug information. The PR also adds CFLAGS=-g in
ocamlest/Makefile to fix this.