Skip to content

Commit

Permalink
Test for absence of select: introduce open_1024 in tests
Browse files Browse the repository at this point in the history
We'll check statically that we are not using Unix.select, but it is good to have a runtime check too,
in case some library (C or OCaml) indirectly uses it.
Open 1024 file descriptors at the beginning of the test, so that we would hit an exception on the first use of Unix.select with any newly created
file descriptor as their value would be >1024 (the limit in `select`).

Unconditionally call these in the stdext-unix, quicktest, and xapi tests.
Set the default ulimit in 'make test' to 2048.

If you are running tests with `dune runtest` directly then you might need to raise this limit,
if your shell has a lower limit by default.
If needed we could add a better error message or a check for ulimit before attempting to open the 1024 files.

No functional change for the product.

Signed-off-by: Edwin Török <[email protected]>
  • Loading branch information
edwintorok committed May 30, 2024
1 parent 00ec519 commit 3cfff61
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 0 deletions.
1 change: 1 addition & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ TEST_TIMEOUT=600
TEST_TIMEOUT2=1200
test:
ulimit -S -t $(TEST_TIMEOUT); \
ulimit -n 2048; \
(sleep $(TEST_TIMEOUT) && ps -ewwlyF --forest)& \
PSTREE_SLEEP_PID=$$!; \
trap "kill $${PSTREE_SLEEP_PID}" SIGINT SIGTERM EXIT; \
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,4 +300,5 @@ let tests =
let () =
(* avoid SIGPIPE *)
let (_ : Sys.signal_behavior) = Sys.signal Sys.sigpipe Sys.Signal_ignore in
Xapi_stdext_unix.Unixext.test_open 1024;
QCheck_base_runner.run_tests_main tests
17 changes: 17 additions & 0 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.ml
Original file line number Diff line number Diff line change
Expand Up @@ -913,6 +913,23 @@ let domain_of_addr str =
Some (Unix.domain_of_sockaddr (Unix.ADDR_INET (addr, 1)))
with _ -> None
let test_open_called = Atomic.make false
let test_open n =
if not (Atomic.compare_and_set test_open_called false true) then
invalid_arg "test_open can only be called once" ;
(* we could make this conditional on whether ulimit was increased or not,
but that could hide bugs if we think the CI has tested this, but due to ulimit it hasn't.
*)
if n > 0 then (
let socket = Unix.socket Unix.PF_UNIX Unix.SOCK_STREAM 0 in
at_exit (fun () -> Unix.close socket) ;
for _ = 2 to n do
let fd = Unix.dup socket in
at_exit (fun () -> Unix.close fd)
done
)
module Direct = struct
type t = Unix.file_descr
Expand Down
11 changes: 11 additions & 0 deletions ocaml/libs/xapi-stdext/lib/xapi-stdext-unix/unixext.mli
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,17 @@ val statvfs : string -> statvfs_t
val domain_of_addr : string -> Unix.socket_domain option
(** Returns Some Unix.PF_INET or Some Unix.PF_INET6 if passed a valid IP address, otherwise returns None. *)

val test_open : int -> unit
(** [test_open n] opens n file descriptors. This is useful for testing that the application makes no calls
to [Unix.select] that use file descriptors, because such calls will then immediately fail.
This assumes that [ulimit -n] has been suitably increased in the test environment.
Can only be called once in a program, and will raise an exception otherwise.
The file descriptors will stay open until the program exits.
*)

module Direct : sig
(** Perform I/O in O_DIRECT mode using 4KiB page-aligned buffers *)

Expand Down
1 change: 1 addition & 0 deletions ocaml/quicktest/quicktest.ml
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

let () =
Quicktest_args.parse () ;
Xapi_stdext_unix.Unixext.test_open 1024;
Qt_filter.wrap (fun () ->
let suite =
[
Expand Down
2 changes: 2 additions & 0 deletions ocaml/tests/common/suite_init.ml
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
let harness_init () =
(* before any calls to XAPI code, to catch early uses of Unix.select *)
Xapi_stdext_unix.Unixext.test_open 1024;
Xapi_stdext_unix.Unixext.mkdir_safe Test_common.working_area 0o755 ;
(* Alcotest hides the standard output of successful tests,
so we will probably not exceed the 4MB limit in Travis *)
Expand Down

0 comments on commit 3cfff61

Please sign in to comment.