Skip to content

Commit c258a12

Browse files
mrochfacebook-github-bot
authored andcommitted
catch EBADF exceptions from Lwt_process.with_process_full
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
1 parent f8cbad1 commit c258a12

File tree

1 file changed

+28
-2
lines changed

1 file changed

+28
-2
lines changed

src/common/lwt/lwtSysUtils.ml

+28-2
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,37 @@ let prepare_args cmd args =
5050
instead, when we pass "". *)
5151
("", Array.of_list (cmd :: args))
5252

53+
(** At least as of Lwt 5.5.0, [Lwt_process.with_process_full] tries to close
54+
the process even when [f] fails, and can raise an EBADF that swallows
55+
whatever the original exception was. https://github.com/ocsigen/lwt/issues/956
56+
57+
Instead, we will swallow exceptions from [close] and use our [Exception] to
58+
reraise the original exception. We also use ppx_lwt instead of [Lwt.finalize]
59+
to improve backtraces. *)
60+
let with_process_full ?timeout ?env ?cwd cmd f =
61+
let process = Lwt_process.open_process_full ?timeout ?env ?cwd cmd in
62+
let ignore_close process =
63+
try%lwt
64+
let%lwt _ = process#close in
65+
Lwt.return_unit
66+
with
67+
| Unix.Unix_error (Unix.EBADF, _, _) -> Lwt.return_unit
68+
in
69+
let%lwt result =
70+
try%lwt f process with
71+
| e ->
72+
let exn = Exception.wrap e in
73+
let%lwt () = ignore_close process in
74+
Exception.reraise exn
75+
in
76+
let%lwt () = ignore_close process in
77+
Lwt.return result
78+
5379
let exec ?env ?cwd cmd args =
54-
Lwt_process.with_process_full ?env ?cwd (prepare_args cmd args) command_result_of_process
80+
with_process_full ?env ?cwd (prepare_args cmd args) command_result_of_process
5581

5682
let exec_with_timeout ~timeout cmd args =
57-
Lwt_process.with_process_full (prepare_args cmd args) (fun process ->
83+
with_process_full (prepare_args cmd args) (fun process ->
5884
let timeout_msg =
5985
Printf.sprintf "Timed out while running `%s` after %.3f seconds" cmd timeout
6086
in

0 commit comments

Comments
 (0)