Skip to content

Commit

Permalink
catch EBADF exceptions from Lwt_process.with_process_full
Browse files Browse the repository at this point in the history
Summary:
we're seeing exceptions like this:

```
Failed to initialize watchman: Unix.Unix_error(Unix.EBADF, "close", "")
Raised by primitive operation at Lwt_unix.self_result in file "src/unix/lwt_unix.cppo.ml", line 237, characters 14-31
Re-raised at Watchman.get_sockname.(fun) in file "flow/src/hack_forked/watchman/watchman.ml", line 381, characters 2-934
Re-raised at FileWatcher.WatchmanFileWatcher.watchman#wait_for_init.go_exn.(fun) in file "flow/src/monitor/fileWatcher.ml", line 534, characters 10-1023
```

`Watchman.get_sockname` calls `LwtSysUtils.exec` which calls `Lwt_process.with_process_full`. `with_process_full` does an `Lwt.finally` to make sure it calls `process#close`. `close` is throwing because the process's fd is invalid (perhaps already closed).

The stack traces aren't much to go on, but I suspect this is happening on exceptions. The file descriptor shouldn't be invalid otherwise. That means there's an underlying exception that's getting hidden by the `finally` also throwing.

So, here we ignore `EBADF` exceptions from the `close` and rethrow the original exception.

I also filed ocsigen/lwt#956 to discuss doing this upstream.

Changelog: [internal]

Reviewed By: samwgoldman

Differential Revision: D37420401

fbshipit-source-id: 0e7ddf41a3f4d13290e49c692be66723eda1f227
  • Loading branch information
mroch authored and facebook-github-bot committed Jun 29, 2022
1 parent f8cbad1 commit c258a12
Showing 1 changed file with 28 additions and 2 deletions.
30 changes: 28 additions & 2 deletions src/common/lwt/lwtSysUtils.ml
Original file line number Diff line number Diff line change
Expand Up @@ -50,11 +50,37 @@ let prepare_args cmd args =
instead, when we pass "". *)
("", Array.of_list (cmd :: args))

(** At least as of Lwt 5.5.0, [Lwt_process.with_process_full] tries to close
the process even when [f] fails, and can raise an EBADF that swallows
whatever the original exception was. https://github.com/ocsigen/lwt/issues/956
Instead, we will swallow exceptions from [close] and use our [Exception] to
reraise the original exception. We also use ppx_lwt instead of [Lwt.finalize]
to improve backtraces. *)
let with_process_full ?timeout ?env ?cwd cmd f =
let process = Lwt_process.open_process_full ?timeout ?env ?cwd cmd in
let ignore_close process =
try%lwt
let%lwt _ = process#close in
Lwt.return_unit
with
| Unix.Unix_error (Unix.EBADF, _, _) -> Lwt.return_unit
in
let%lwt result =
try%lwt f process with
| e ->
let exn = Exception.wrap e in
let%lwt () = ignore_close process in
Exception.reraise exn
in
let%lwt () = ignore_close process in
Lwt.return result

let exec ?env ?cwd cmd args =
Lwt_process.with_process_full ?env ?cwd (prepare_args cmd args) command_result_of_process
with_process_full ?env ?cwd (prepare_args cmd args) command_result_of_process

let exec_with_timeout ~timeout cmd args =
Lwt_process.with_process_full (prepare_args cmd args) (fun process ->
with_process_full (prepare_args cmd args) (fun process ->
let timeout_msg =
Printf.sprintf "Timed out while running `%s` after %.3f seconds" cmd timeout
in
Expand Down

0 comments on commit c258a12

Please sign in to comment.