Lwt: Correct way of closing inherited file descriptors in non-lwt child process?

[mjambon]

Our application is structured as a master process with takes care of IOs using Lwt, and child processes which are CPU-intensive and return results to the parent over pipes. So, we have a master process reading data from multiple pipes connected to children processes. When we create a new child worker using Unix.fork, it inherits the parent's file descriptors. The child performs only synchronous IOs. How can we close the various pipes and other files that only the parent should own?

I tried various things without complete success. In the child, closing all file descriptors except the ones we need with Unix.close results in exceptions being raised out of the blue, apparently by GC finalizers. A file descriptor shown as anon_inode:[eventfd] by ls -l /proc/self/fd can't be closed otherwise we get an exception during garbage collection in the child. But this is not enough. I tried registering functions to close file descriptor that was wrapped by Lwt (in an lwt fd or an lwt channel) but I'm still getting Unix.Unix_error(Unix.EBADF, "close", "") somewhere and it's hard to track without knowing which file descriptor failed.

Is there something obvious I'm missing?

(I also tried exiting Lwt_main.run using an exception in the child but this didn't seem to solve my problems and it introduced new ones. Using Lwt_unix.fork instead of Unix.fork didn't change anything at the point where I tried it, and according to the manual it's only needed if we want to use lwt in the child. Right now I register custom close functions for every file descriptor I know about which is managed by lwt. It's slightly cumbersome but reasonable, only it doesn't work; I'd like to know if this worth pursuing. If so, I'd like to know how to close a channel synchronously: Lwt.async (fun () -> Lwt_io.abort lwt_channel) doesn't seem to close the file descriptor.)

[aantron]

(I had a comment about CLOEXEC but I realized immediately (again) that you are not calling exec :p)

[aantron]

It's not clear to me from the text: have you tried closing the Lwt file descriptors using Lwt's own functions, before continuing with the rest of the child, using only non-Lwt I/O? You should prefer Lwt_unix.fork in that case.

[mjambon]

I tried to close with lwt's functions, but I didn't realize that it
effectively meant I was using lwt in the child. I'll try Lwt_unix.fork,
then. Thank you!

On Fri, Oct 11, 2019, 21:49 Anton Bachin [e-mail] wrote:

It's not clear to me from the text: have you tried closing the Lwt file
descriptors using Lwt's own functions, before continuing with the rest of
the child, using only non-Lwt I/O? You should prefer Lwt_unix.fork in
that case.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
https://github.com/ocsigen/lwt/issues/737?email_source=notifications&email_token=AACTZYK73VJ37IJYN7A5F5LQOFJNZA5CNFSM4JAAROOKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEBBVTCY#issuecomment-541284747,
or unsubscribe
https://github.com/notifications/unsubscribe-auth/AACTZYI3ZANY6PCPZ6CCC7LQOFJNZANCNFSM4JAAROOA
.

[mjambon]

I tried using Lwt in the child so as to run a list of functions of type unit -> unit Lwt.t which close my channels and file-like resources. The problem is that the child inherits Lwt tasks from the parent. These tasks should not run in the child (e.g. they may read from a pipe end that belongs to the parent). Is there a way to run a list of functions of type (unit -> unit Lwt.t) list and nothing else? Is there a way to somehow clear the Lwt scheduler?

[mjambon]

The alternative I've been considering if this fails is to exec the program we're already running so as to start from a clean state. Its first job would to be close all file descriptors except those needed, using just Unix.close.

[aantron]

The problem is that the child inherits Lwt tasks from the parent

Do you mean submitted Lwt I/Os that are already running, or the user application's code that may be triggered somehow in the child? If the former, then that should be a bug in Lwt. Lwt seems to make an effort to prevent Lwt I/O from simply continuing in the child:

https://github.com/ocsigen/lwt/blob/336566dd63d7f948234a4f30919df329f25b0e3c/src/unix/lwt_unix.cppo.ml#L2194-L2212

If the latter, that may be a bug in application, but it may be a bug that can't be fixed due to excessive subtlety of combining fork with Lwt. I would need more information to reason about it.

The alternative I've been considering if this fails is to exec the program

I personally would lean towards this, combined with O_CLOEXEC discipline, if possible.

[mjambon]

Do you mean submitted Lwt I/Os that are already running

Yes, that's what I meant. I have a child failing with an error message that's normally produced by the parent. I'll investigate further since Lwt_unix.fork should already prevent this from happening.

[mjambon]

Here's a program that shows tasks from the parent continuing to execute in the child after Lwt_unix.fork: readme source

I haven't looked into Lwt code at this point nor have I tried to reproduce on other platforms than the single combination I happened to be using.

[mjambon]

I think I found the problem, in lwt_io.ml:

let () =
  (* Flush all opened output channels on exit: *)
  Lwt_main.at_exit flush_all

[mjambon]

or maybe not, I'll post a confirmation. Sending sigkill to self instead of running exit 0 or exit 1 gets rid of the problem.

[mjambon]

Here's how far I got: commenting out Lwt.wakeup_paused (); at the beginning of Lwt_main.run makes things behave as expected.

$ git diff
diff --git a/src/unix/lwt_main.ml b/src/unix/lwt_main.ml
index 42435523..d6dedce0 100644
--- a/src/unix/lwt_main.ml
+++ b/src/unix/lwt_main.ml
@@ -22,7 +22,7 @@ let yield () = (Lwt.add_task_r [@ocaml.warning "-3"]) yielded

 let rec run t =
   (* Wakeup paused threads now. *)
-  Lwt.wakeup_paused ();
+  (*Lwt.wakeup_paused ();*)
   match Lwt.poll t with
   | Some x ->
     x

Lwt_main.run itself is invoked as part of an exit hook registered in lwt_main.ml:

let () =
  at_exit (fun () ->
    Lwt.abandon_wakeups ();
    run (call_hooks ()))

I don't know if these exit hooks are necessary or useful after a fork. It includes some that may be registered by the user, and I don't know what the user would expect there.

At this point, I want to imagine that Lwt.wakeup_paused (); should be unproblematic because the call to Lwt_unix.fork takes care of "aborting all pending tasks", but it's not happening properly. I'll try to continue looking into this tomorrow.

[mjambon]

A workaround consisting in bypassing the at_exit hooks is posted here: https://github.com/mjambon/lwt-issue-737/blob/5c935e0380571186fa7f4411d1ce53c6f984e971/src/main.ml#L85-L100

[mjambon]

To recapitulate, I saw two problems:

1. An exception Unix.Unix_error(Unix.EBADF, "send_notification", "") in the child process occurring after closing the eventfd file descriptor. It looks like it's triggered from a GC finalizer. I wasn't able to reproduce this in a small program. The workaround for this seems to be to not close that file descriptor. I can't tell at this point if it's enough.
2. Exceptions at the end of the child due to closed file descriptors, raised by Lwt_main.run which gets executed as part of an exit hook registered by Lwt. A workaround is to bypass the exit hooks by calling sys_exit. Details for this are given in the comments above.

These two issues may or may not have the same root cause. They both involve Lwt tasks that should not run in the child process after Lwt_unix.fork ().

[aantron]

Thanks. I just took a brief fresh look at the issue, and I hope to debug it in detail in the coming days.

[aantron]

@mjambon, the above commit tweaks Lwt_main's process exit hook not to call Lwt_main.run if there are no Lwt exit hooks. This allows you to fix the immediate issue by removing Lwt exit hooks after fork, using an existing API:

match Lwt_unix.fork () with
| 0 ->
  Lwt_main.Exit_hooks.remove_all ();
  (* ... *)

As to the underlying problem, it seems difficult to decide what to do, which I think you also pointed out in an earlier comment. We can't easily know if the child processes will use Lwt, Lwt_io, whether they will have code left over from the parent process that is already doing I/O with Lwt_io, or whether the child needs to preserve exit hooks. The docs of Lwt_unix.fork do try to help the user call the right functions around fork, so I edited those docs.

The other issue, with the eventfd file descriptor, almost certainly has to do with the internal fd used by worker threads in Lwt_unix to communicate with the main thread. You may be able to make progress debugging that by adding Lwt to a Dune workspace with your project, and instrumenting usage of that fd. It is declared here:

https://github.com/ocsigen/lwt/blob/abc445f587a36e2084c081166a6a98557ab5f42b/src/unix/lwt_unix_stubs.c#L650

and initialized here:

https://github.com/ocsigen/lwt/blob/abc445f587a36e2084c081166a6a98557ab5f42b/src/unix/lwt_unix_stubs.c#L700

[mjambon]

Thank you very much, @aantron.

Here's a quick follow-up:

• Bypassing the exit hooks in our application has been successful. With your fix, calling Lwt_main.Exit_hooks.remove_all () will be sufficient, which is great.
• We also started tracking all file descriptors that our application created. In the child, we only close those, which leaves the eventfd file descriptor open. With this, we no longer get EBADF errors.

As a side note, tracking file descriptors in a global table is nice since we can give them names as well as unique (non-recycled) identifiers. This allows us to list them and makes it easy to troubleshoot suspected FD leaks. As a complement, inspecting /proc/PID/fd on Linux is useful to find out about FDs that we're not tracking.

[mjambon]

We may want to keep this ticket open (or create a new one) regarding the EBADF errors occurring after closing eventfd. This doesn't seem to be a problem at the moment for our application as long as we don't close it. Closing it with any other file descriptor (other than stdin, stdout, stderr) from a new program after an exec is unproblematic.

[aantron]

I would suggest to close this issue, and open a new one when there is more information towards addressing the EBADF.