From 31b575092cb1da0b1ae6b118eb71a0f5a6d52b70 Mon Sep 17 00:00:00 2001 From: Erik Martin-Dorel Date: Wed, 8 Sep 2021 17:42:37 +0200 Subject: [PATCH 1/4] test: Add -j 2 in corpus tests --- tests/runtests.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/runtests.sh b/tests/runtests.sh index c1f06b506..8674e6a6e 100755 --- a/tests/runtests.sh +++ b/tests/runtests.sh @@ -234,7 +234,7 @@ do if ! ( set -x; docker run --entrypoint '' \ -v "$(realpath "$corpus"):/repository" \ - learn-ocaml /bin/sh -c "learn-ocaml --repo=/repository build" ); then + learn-ocaml /bin/sh -c "learn-ocaml --repo=/repository build -j 2" ); then red "Failed to build $corpus" exit 1 fi From 86987534c9450dc1a015613960d272cf1f5192b5 Mon Sep 17 00:00:00 2001 From: Erik Martin-Dorel Date: Wed, 8 Sep 2021 18:18:28 +0200 Subject: [PATCH 2/4] fix(Lwt_utils.mkdir_p): Surround the Lwt_unix.mkdir call with a Lwt.catch guard Aim: mitigate a TOC/TOU race condition which occurs when Learnocaml_process_exercise_repository.n_processes > 1, triggered by a command such as "learn-ocaml build -j 2". This isn't the only possible fix/workaround, just a concise one relying on error handling. --- src/utils/lwt_utils.ml | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/utils/lwt_utils.ml b/src/utils/lwt_utils.ml index 0fe27a22c..537f72e61 100644 --- a/src/utils/lwt_utils.ml +++ b/src/utils/lwt_utils.ml @@ -18,7 +18,14 @@ let rec mkdir_p ?(perm=0o755) dir = (Printf.sprintf "Can't create dir: file %s is in the way" dir) | false -> mkdir_p (Filename.dirname dir) >>= fun () -> - Lwt_unix.mkdir dir perm + (* This exn handler mitigates a TOC/TOU race condition which + occurs when [mkdir_p] is called from parallel build jobs + (namely, for some "learn-ocaml build -j 2" command) *) + Lwt.catch (fun () -> Lwt_unix.mkdir dir perm) + (function + | Unix.Unix_error(Unix.EEXIST, "mkdir", _path) + when Sys.is_directory dir -> Lwt.return () + | e -> Lwt.fail e) let copy_file src dst = Lwt.catch (fun () -> From e0e277dcc06e22c7d6c9298fce63aa8593ecf782 Mon Sep 17 00:00:00 2001 From: Erik Martin-Dorel Date: Wed, 8 Sep 2021 18:51:03 +0200 Subject: [PATCH 3/4] Revert "fix(Lwt_utils.mkdir_p): Surround the Lwt_unix.mkdir call with a Lwt.catch guard" This reverts commit 86987534c9450dc1a015613960d272cf1f5192b5. --- src/utils/lwt_utils.ml | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/utils/lwt_utils.ml b/src/utils/lwt_utils.ml index 537f72e61..0fe27a22c 100644 --- a/src/utils/lwt_utils.ml +++ b/src/utils/lwt_utils.ml @@ -18,14 +18,7 @@ let rec mkdir_p ?(perm=0o755) dir = (Printf.sprintf "Can't create dir: file %s is in the way" dir) | false -> mkdir_p (Filename.dirname dir) >>= fun () -> - (* This exn handler mitigates a TOC/TOU race condition which - occurs when [mkdir_p] is called from parallel build jobs - (namely, for some "learn-ocaml build -j 2" command) *) - Lwt.catch (fun () -> Lwt_unix.mkdir dir perm) - (function - | Unix.Unix_error(Unix.EEXIST, "mkdir", _path) - when Sys.is_directory dir -> Lwt.return () - | e -> Lwt.fail e) + Lwt_unix.mkdir dir perm let copy_file src dst = Lwt.catch (fun () -> From e191f59fa25b5fb3e45d462999bbe1a533e4b25b Mon Sep 17 00:00:00 2001 From: Erik Martin-Dorel Date: Wed, 8 Sep 2021 18:57:03 +0200 Subject: [PATCH 4/4] fix: some TOC/TOU race condition when running "learn-ocaml build -j 2" Instead of tweaking Lwt_utils.mkdir_p, we add a call to this function for the "www/static" directory before running the parallel build jobs. MINOR-DRAWBACK: "www/static" is created even if there is no exercise! --- src/repo/learnocaml_process_exercise_repository.ml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/repo/learnocaml_process_exercise_repository.ml b/src/repo/learnocaml_process_exercise_repository.ml index df2cbb627..43c964c3d 100644 --- a/src/repo/learnocaml_process_exercise_repository.ml +++ b/src/repo/learnocaml_process_exercise_repository.ml @@ -230,6 +230,8 @@ let main dest_dir = Lwt_list.map_p, spawn_grader in + let static_dir = String.concat Filename.dir_sep [dest_dir; "static"] in + Lwt_utils.mkdir_p static_dir >>= fun () -> listmap (fun (id, ex_dir, exercise, json_path, changed, dump_outputs,dump_reports) -> let dst_ex_dir = String.concat Filename.dir_sep [dest_dir; "static"; id] in Lwt_utils.mkdir_p dst_ex_dir >>= fun () ->