Skip to content

Commit

Permalink
[cache] Fix dune freezing on MacOS with cache enabled
Browse files Browse the repository at this point in the history
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 ocaml#3233 is now gone.
3. Still works fine on Linux.

Fixes ocaml#3233 ocaml#2973

Signed-off-by: Artem Pyanykh <artem.pyanykh@gmail.com>
  • Loading branch information
artempyanykh committed Mar 9, 2020
1 parent 39796ab commit b28043c
Show file tree
Hide file tree
Showing 3 changed files with 4 additions and 6 deletions.
5 changes: 2 additions & 3 deletions src/cache_daemon/cache_daemon.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions test/blackbox-tests/dune.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion test/blackbox-tests/gen_tests.ml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit b28043c

Please sign in to comment.