From f1fa7b3236bf773481a42987a6648e1cc326fc4e Mon Sep 17 00:00:00 2001 From: Artem Pyanykh Date: Mon, 9 Mar 2020 08:49:15 +0000 Subject: [PATCH] [cache] Fix dune freezing on MacOS with cache enabled Summary: On MacOS shutdown on a socket raises `ENOTCONN` even when only one side of the duplex communication is closed. This doesn't make much sense to me (also, man pages are rather fuzzy around this, and on Linux it behaves differently), but it is what it is. Since cache/client calls `Unix.shutdown client.fd Unix.SHUTDOWN_SEND` during teardown, `cache_daemon.ml` gets an exception on its shutdown, which it swallows and therefore never closes `client.fd`. Test plan: 1. Cache trimming tests work (and now enabled for all platforms), 2. Freezing described in #3233 is now gone. 3. Still works fine on Linux. Fixes #3233 #2973 --- src/cache_daemon/cache_daemon.ml | 5 ++--- test/blackbox-tests/dune.inc | 3 +-- test/blackbox-tests/gen_tests.ml | 2 +- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/cache_daemon/cache_daemon.ml b/src/cache_daemon/cache_daemon.ml index 5b5c3099b6d3..a02e87911f6f 100644 --- a/src/cache_daemon/cache_daemon.ml +++ b/src/cache_daemon/cache_daemon.ml @@ -181,10 +181,9 @@ let client_thread (events, (client : client)) = in handle client and finally () = - ( try - Unix.shutdown client.fd Unix.SHUTDOWN_ALL; - Unix.close client.fd + ( try Unix.shutdown client.fd Unix.SHUTDOWN_ALL with Unix.Unix_error (Unix.ENOTCONN, _, _) -> () ); + Unix.close client.fd; Evt.sync (Evt.send events (Client_left client.fd)) in try Exn.protect ~f ~finally with diff --git a/test/blackbox-tests/dune.inc b/test/blackbox-tests/dune.inc index bd6dcded5406..b2934938e6a3 100644 --- a/test/blackbox-tests/dune.inc +++ b/test/blackbox-tests/dune.inc @@ -358,8 +358,7 @@ (action (chdir test-cases/dune-cache/trim - (progn (run dune-cram run run.t) (diff? run.t run.t.corrected)))) - (enabled_if (<> %{ocaml-config:system} macosx))) + (progn (run dune-cram run run.t) (diff? run.t run.t.corrected))))) (rule (alias dune-init) diff --git a/test/blackbox-tests/gen_tests.ml b/test/blackbox-tests/gen_tests.ml index d96e0a94395d..e1c1211e1e11 100644 --- a/test/blackbox-tests/gen_tests.ml +++ b/test/blackbox-tests/gen_tests.ml @@ -221,7 +221,7 @@ let exclusions = ; make "merlin/merlin-tests" ~external_deps:true ; make "use-meta" ~external_deps:true ; make "output-obj" ~skip_platforms:[ Mac; Win ] ~only_ocaml:(">=", "4.06.0") - ; make "dune-cache/trim" ~skip_platforms:[ Mac ] + ; make "dune-cache/trim" ; make "github644" ~external_deps:true ; make "private-public-overlap" ~external_deps:true ; make "reason" ~external_deps:true