Lwt: Lwt_process.unix_spawn should use unix_exit not caml_sys_exit

[ljw1004]

When we call Lwt_process.open_process it calls unix_spawn, which does Lwt_unix.fork then Unix.execvp then (in case of failure, e.g. the executable couldn't be found) it calls extern function caml_sys_exit 127. (The use of caml_sys_exit was introduced in 2014 as a robust way to stop any ocaml-registered exit hooks from running - https://github.com/ocsigen/lwt/commit/0f70012a1cd1daf3a17c0a3451d2c762d206d0be)

ISSUE:

We also need to stop other exit-hooks from running, e.g. those written in C. To do this we should be using unix_exit instead of caml_sys_exit.

(* this will stop ocaml exit hooks from running, but will leave other hooks *)
external sys_exit : int -> 'a = "caml_sys_exit"

(* this will stop ALL exit hooks from running. *)
external unix_exit : int -> 'a = "unix_exit"

REPRO:

Sorry, this is useless of me, but I don't have a standalone small repro. I'm hooked into part of a bigger project, which is using some great big written-in-C library that installs its own exit hooks. I don't know the C APIs for registering exit hooks. But what I observed was that, when I invoked Lwt_process.open_process to launch a non-existent executable, then it forked a PID and this PID was just hung indefinitely, and pstack <forked_pid> showed this:

...
#10 0x000000000cc84a29 in <great big C library's exit handler>
#11 0x00007f36d214c83a in __run_exit_handlers () from /usr/local/fbcode/platform007/lib/libc.so.6
#12 0x00007f36d214c88a in exit () from /usr/local/fbcode/platform007/lib/libc.so.6
#13 0x000000000e06ec15 in caml_sys_exit (retcode_v=<optimized out>) at sys.c:159
#14 0x00000000041e72c2 in camlLwt_process__unix_spawn_1892 () at src/unix/lwt_process.ml:168
#15 0x00000000041e7f28 in camlLwt_process__fun_3762 () at src/unix/lwt_process.ml:312
...

I next made a repro which didn't use Lwt_process.open_process at all, and instead just invoked Unix.fork, Unix.execvp, caml_sys_exit (i.e. the same as Lwt_process.open_process does), and observed that this reproduced the exact same hang.

I next changed it to use unix_exit instead of caml_sys_exit and observed that it didn't hang.

NOTE: as I said, I don't understand much about hooks. It was @samwgoldman who advised me that caml_sys_exit looked wrong, and who suggested unix_exit, and who suggested I post this issue here for the attention of @aantron.

[aantron]

Thanks, I just want to confirm that I see the issue and it does seem reasonable to me after an initial read. I will look at it in greater detail in the first days of next week.

[ljw1004]

I made a minimal standalone repro.

dune

(library
  (modules )
  (name foreign)
  (c_names foreign)
  (preprocess (pps lwt_ppx)))

(executable
  (name repro)
  (modules repro)
  (libraries foreign lwt lwt.unix)
  (preprocess (pps lwt_ppx)))

repro.ml

external init_c : unit -> unit = "init_c"

let f () : unit Lwt.t =
  let process = Lwt_process.open_process_in ("", [| "not_echo"; "a" |]) in
  let%lwt stdout = Lwt_io.read_line process#stdout in
  let%lwt _status = process#close in
  let%lwt () = Lwt_io.printf "done. stdout=%s.\n%!" stdout in
  Lwt.return_unit

let () =
  init_c ();
  Lwt_main.run (f ())

foreign.c

#define CAML_NAME_SPACE
#include <caml/mlvalues.h>
#include <caml/unixsupport.h>
#include <caml/memory.h>
#include <caml/alloc.h>
#include <caml/fail.h>
#undef CAML_NAME_SPACE

#include <stdio.h>
#include <string.h>
#include <sys/types.h>
#include <unistd.h>

pid_t init_pid;

void my_atexit(void) {
  pid_t current_pid=getpid();
  fprintf(stderr,"my_atexit. init_pid=%d current_pid=%d\n",init_pid,current_pid);
  if (init_pid == current_pid) return;
  fprintf(stderr,"my_atexit: deadlock because running in wrong pid\n");
  while (1) ;
}

void my_onexit(int ev, void *arg)
{
  pid_t current_pid=getpid();
  fprintf(stderr,"my_onexit. init_pid=%d current_pid=%d\n",init_pid,current_pid);
  if (init_pid == current_pid) return;
  fprintf(stderr,"my_onexit: deadlock because running in wrong pid\n");
  while (1) ;
}

void init_c(void) {
  CAMLparam0();
  init_pid=getpid();
  atexit(my_atexit);
  on_exit(my_onexit, 0);
  CAMLreturn0;
}

What I observe

When launched, this program prints the following and hangs indefinitely:

my_onexit. init_pid=2504728 current_pid=2504729
my_onexit: deadlock because running in wrong pid

$ pstack 2504729
#0  my_onexit (ev=<optimized out>, arg=<optimized out>) at foreign.c:30
#1  my_onexit (ev=<optimized out>, arg=<optimized out>) at foreign.c:24
#2  0x00007f0c60b82b49 in __run_exit_handlers () from /lib64/libc.so.6
#3  0x00007f0c60b82bb7 in exit () from /lib64/libc.so.6
#4  0x00000000004e79f2 in caml_sys_exit (retcode_v=<optimized out>) at sys.c:159
#5  0x000000000046ceea in camlLwt_process__unix_spawn_1885 () at src/unix/lwt_process.ml:163
#6  0x000000000046d3f2 in camlLwt_process__fun_3674 () at src/unix/lwt_process.ml:269
#7  0x0000000000453892 in camlRepro__f_1003 () at src/unix/lwt_process.ml:329
#8  0x0000000000453ab6 in camlRepro__entry () at repro/repro.ml:12
#9  0x0000000000450c39 in caml_program ()

If you remove the call to init_c, or if you change it to invoke echo rather than (the deliberately not found) not_echo, then works fine.

Discussion

I encountered this when using https://github.com/facebook/folly from my ocaml code. That library uses threads internally, and it also installs an exit handler to make sure the threads are cleaned up. If the exit handler is run from a forked PID, then it deadlocks (presumably because of mutex ownership issues of some sort).

In the repro, I wrote out on_exit and atexit handlers for completeness. There's nothing special about either; each of them alone is sufficient to cause the deadlock.

It would be possible for users to avoid this by using pthread_atfork to disable exit handlers in the child process. That said, there's no way to actually remove exit handlers, so I'd need to go through all the libraries I'm using, and figure out their particular semantics for rendering inert their particular behavior. Indeed one stackoverflow answer to the question "how do I remove exit handlers?" is "don't; use Exit instead" https://stackoverflow.com/questions/2310673/can-i-undo-or-remove-an-atexit-command

Here is a demonstration that caml_sys_exit will invoke exit handlers, but unix_exit won't:

external init_c : unit -> unit = "init_c"
external caml_sys_exit : int -> 'a = "caml_sys_exit"
external unix_exit : int -> 'a = "unix_exit"

let () =
  init_c ();
  unix_exit 127

If you run this, it prints nothing. But if you change it to invoke caml_sys_exit then it will run the exit handlers.

The general unix guidance is that the only safe thing to do after a fork is an exec. I guess that calling caml_sys_exit is just one example of a thing that's not safe after a fork...

[aantron]

Thanks for making that so easy to fix! I'll release this soon, after also addressing #761.

[aantron]

This has been released in 5.1.2.