Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Teardown does not work on OSX #2973

Closed
rgrinberg opened this issue Dec 14, 2019 · 4 comments
Closed

Teardown does not work on OSX #2973

rgrinberg opened this issue Dec 14, 2019 · 4 comments
Assignees

Comments

@rgrinberg
Copy link
Member

The following test (dune-cache-trimming/run.t) never terminates (consistently) for me on Mac:

Check background trimming.

  $ env DUNE_CACHE=enabled DUNE_CACHE_TRIM_SIZE=1 DUNE_CACHE_TRIM_PERIOD=1 XDG_RUNTIME_DIR=$PWD/.xdg-runtime XDG_CACHE_HOME=$PWD/.xdg-cache dune cache start > /dev/null 2>&1
  $ env DUNE_CACHE=enabled DUNE_CACHE_EXIT_NO_CLIENT=1 XDG_RUNTIME_DIR=$PWD/.xdg-runtime XDG_CACHE_HOME=$PWD/.xdg-cache dune build target_a
  $ rm -f _build/default/target_a _build/default/beacon_a
  $ sleep 2
  $ env DUNE_CACHE=enabled DUNE_CACHE_EXIT_NO_CLIENT=1 XDG_RUNTIME_DIR=$PWD/.xdg-runtime XDG_CACHE_HOME=$PWD/.xdg-cache dune build target_a
  $ test -e _build/default/beacon_a
  $ env DUNE_CACHE=enabled XDG_RUNTIME_DIR=$PWD/.xdg-runtime XDG_CACHE_HOME=$PWD/.xdg-cache dune cache stop

The following call to teardown triggers a clean up process:

  | Some { cache = (module Caching : Dune_cache.Caching); _ } ->
    (* Synchronously wait for the end of the connection with the cache daemon,
       ensuring all dedup messages have been queued. *)
    Caching.Cache.teardown Caching.cache;

That doesn't seem to terminate. This shutdown call:

  let teardown client =
    ( try Unix.shutdown client.fd Unix.SHUTDOWN_SEND
      with Unix.Unix_error (Unix.ENOTCONN, _, _) -> () );
    Thread.join client.thread

Never seems to terminate because the client thread is stuck reading:

  thread #5
    frame #0: 0x00007fff64f50c7e libsystem_kernel.dylib`read + 10
    frame #1: 0x0000000108927fb3 dune`caml_read_fd(fd=4, flags=<unavailable>, buf=0x0000000109565050, n=<unavailable>) at unix.c:79:15 [opt]
    frame #2: 0x000000010891dacb dune`caml_ml_input(vchannel=4449792192, buff=<unavailable>, vstart=<unavailable>, vlength=<unavailable>) at io.c:774:13 [opt]
    frame #3: 0x00000001088b44af dune`camlStdlib__input_264 + 79
    frame #4: 0x00000001088cca48 dune`camlStdlib__stream__fill_buff_121 + 56
    frame #5: 0x00000001088ccf51 dune`camlStdlib__stream__peek_data_223 + 497
    frame #6: 0x00000001088a72a2 dune`camlStdune__Csexp__peek_204 + 18
    frame #7: 0x00000001088a7667 dune`camlStdune__Csexp__parse_451 + 103
    frame #8: 0x0000000108860ea7 dune`camlDune_cache_daemon__read_2792 + 103
    frame #9: 0x000000010886361f dune`camlDune_cache_daemon__thread_3571 + 111
    frame #10: 0x000000010886772b dune`camlThread__fun_286 + 43
    frame #11: 0x0000000108931478 dune`caml_start_program + 92
    frame #12: 0x0000000108905da8 dune`caml_thread_start + 104
    frame #13: 0x00007fff65013e65 libsystem_pthread.dylib`_pthread_start + 148
    frame #14: 0x00007fff6500f83b libsystem_pthread.dylib`thread_start + 15
@mefyl
Copy link
Collaborator

mefyl commented Jan 7, 2020

The other side (cache daemon) is supposed to perform a SHUTDOWN_SEND once it's done sending deduplication messages, bringing that read to an end. Can I assume the client is stuck in the Thread.join (and thus the read in the other thread), not the Unix.shutdown ?

@mefyl
Copy link
Collaborator

mefyl commented Jan 7, 2020

In any case Dune should probably have a timeout there to protect itself too.

@rgrinberg
Copy link
Member Author

Can I assume the client is stuck in the Thread.join (and thus the read in the other thread), not the Unix.shutdown ?

Yes, I omitted that but that's exactly what's happening

artempyanykh added a commit to artempyanykh/dune that referenced this issue Mar 9, 2020
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
artempyanykh added a commit to artempyanykh/dune that referenced this issue Mar 9, 2020
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 Pianykh <artem.pyanykh@gmail.com>
artempyanykh added a commit to artempyanykh/dune that referenced this issue Mar 9, 2020
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>
@mefyl
Copy link
Collaborator

mefyl commented Mar 9, 2020

Fixed by #3249 .

@mefyl mefyl closed this as completed Mar 9, 2020
rgrinberg added a commit to rgrinberg/opam-repository that referenced this issue Apr 10, 2020
…lugin, dune-private-libs and dune-glob (2.5.0)

CHANGES:

- Add a `--release` option meaning the same as `-p` but without the
  package filtering. This is useful for custom `dune` invocation in opam
  files where we don't want `-p` (ocaml/dune#3260, @diml)

- Fix a bug introduced in 2.4.0 causing `.bc` programs to be built
  with `-custom` by default (ocaml/dune#3269, fixes ocaml/dune#3262, @diml)

- Allow contexts to be defined with local switches in workspace files (ocaml/dune#3265,
  fix ocaml/dune#3264, @rgrinberg)

- Delay expansion errors until the rule is used to build something (ocaml/dune#3261, fix
  ocaml/dune#3252, @rgrinberg, @diml)

- [coq] Support for theory dependencies and compositional builds using
  new field `(theories ...)` (ocaml/dune#2053, @ejgallego, @rgrinberg)

- From now on, each version of a syntax extension must be explicitely tied to a
  minimum version of the dune language. Inconsistent versions in a
  `dune-project` will trigger a warning for version <=2.4 and an error for
  versions >2.4 of the dune language. (ocaml/dune#3270, fixes ocaml/dune#2957, @voodoos)

- [coq] Bump coq lang version to 0.2. New coq features presented this release
  require this version of the coq lang. (ocaml/dune#3283, @ejgallego)

- Prevent installation of public executables disabled using the `enabled_if` field.
  Installation will now simply skip such executables instead of raising an
  error. (ocaml/dune#3195, @voodoos)

- `dune upgrade` will now try to upgrade projects using versions <2.0 to version
  2.0 of the dune language. (ocaml/dune#3174, @voodoos)

- Add a `top` command to integrate dune with any toplevel, not just
  utop. It is meant to be used with the new `#use_output` directive of
  OCaml 4.11 (ocaml/dune#2952, @mbernat, @diml)

- Allow per-package `version` in generated `opam` files (ocaml/dune#3287, @toots)

- [coq] Introduce the `coq.extraction` stanza. It can be used to extract OCaml
  sources (ocaml/dune#3299, fixes ocaml/dune#2178, @rgrinberg)

- Load ppx rewriters in dune utop and add pps field to toplevel stanza. Ppx
  extensions will now be usable in the toplevel
  (ocaml/dune#3266, fixes ocaml/dune#346, @stephanieyou)

- Add a `(subdir ..)` stanza to allow evaluating stanzas in sub directories.
  (ocaml/dune#3268, @rgrinberg)

- Fix a bug preventing one from running inline tests in multiple modes
  (ocaml/dune#3352, @diml)

- Allow the use of the `%{profile}` variable in the `enabled_if` field of the
  library stanza. (ocaml/dune#3344, @mrmr1993)

- Allow the use of `%{ocaml_version}` variable in `enabled_if` field of the
  library stanza. (ocaml/dune#3339, @voodoos)

- Fix dune build freezing on MacOS when cache is enabled. (ocaml/dune#3249, fixes #ocaml/dune#2973,
  @artempyanykh)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants