Permalink
Browse files

Fix leaking FD when Process times out

Summary:
rFBS5b769b432244e8f17704177ea2706d1884160629 enhanced the unit tests
for process to catch leaking File Descriptors.

It was still leaking for test_process_timeout

This fixes that.

When the process times out, kill the subprocess and close the
File Descriptors.

Reviewed By: dabek

Differential Revision: D6920353

fbshipit-source-id: 597ed5a7cb3cb5cd7c6fefdc1fd84c8faa3ad396
  • Loading branch information...
alexchow authored and hhvm-bot committed Feb 9, 2018
1 parent 7fcd844 commit 72528240c8c62284003d1f1731e56c1db95982a2
@@ -113,6 +113,14 @@ let is_ready process =
| Process_aborted Input_too_large
| Process_exited _ -> true
let kill_and_cleanup_fds pid fds =
Unix.kill pid Sys.sigkill;
let maybe_close fd_ref = Option.iter !fd_ref ~f:begin fun fd ->
Unix.close fd;
fd_ref := None
end in
List.iter fds ~f:maybe_close
(**
* Consumes from stdout and stderr pipes and waitpids on the process.
* Returns immediately if process has already been waited on (so this
@@ -166,6 +174,7 @@ let rec read_and_wait_pid ~retries process =
match Unix.waitpid [Unix.WNOHANG] pid with
| 0, _ ->
if retries <= 0 then
let () = kill_and_cleanup_fds pid [stdout_fd; stderr_fd] in
Error (Timed_out
((Stack.merge_bytes acc), (Stack.merge_bytes acc_err)))
else
@@ -45,4 +45,5 @@ let for_all_non_shortcircuit tests f =
List.fold_left tests ~init:true ~f:(fun acc test -> (f test) && acc)
let run_all (tests: (string * (unit -> bool)) list ) =
Printexc.record_backtrace true;
exit (if for_all_non_shortcircuit tests run then 0 else 1)
@@ -148,22 +148,17 @@ let int_of_fd (x : Unix.file_descr) : int = Obj.magic x
let close_fds fds =
List.iter Unix.close fds
(** Returns true if the next opened file descriptor is exactly 1 greater
(** Asserts the next opened file descriptor is exactly 1 greater
* than the "last_fd". Repeats this "repeats" times. Accumulates all opened
* FDs into all_fds and closes all of them at the end. *)
let rec is_next_fd_incremental ~repeats all_fds last_fd =
let rec assert_next_fd_incremental ~repeats all_fds last_fd =
if repeats <= 0 then
let () = close_fds all_fds in
true
close_fds all_fds
else
let fd = open_an_fd () in
try
let () = Int_asserter.assert_equals ((int_of_fd last_fd) + 1) (int_of_fd fd)
"Test unexpectedly leaked a File Descriptor." in
is_next_fd_incremental ~repeats:(repeats - 1) (fd :: all_fds) fd
with
| Assert_failure _ ->
false
let () = Int_asserter.assert_equals ((int_of_fd last_fd) + 1) (int_of_fd fd)
"Test unexpectedly leaked a File Descriptor." in
assert_next_fd_incremental ~repeats:(repeats - 1) (fd :: all_fds) fd
(** Opens one FD before running a test, and then opens 1000 FDs after the test
* and asserts that all the FD numbers are sequential (i.e. no holes).
@@ -178,8 +173,8 @@ let rec is_next_fd_incremental ~repeats all_fds last_fd =
let assert_no_fd_leaked test = fun () ->
let first_fd = open_an_fd () in
let result = test () in
let incremental = is_next_fd_incremental ~repeats:1000 [first_fd] first_fd in
incremental && result
assert_next_fd_incremental ~repeats:1000 [first_fd] first_fd;
result
let tests = [
("test_echo", assert_no_fd_leaked test_echo);

0 comments on commit 7252824

Please sign in to comment.