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

Cabal run leaves children running when killed #7914

Closed
robx opened this issue Jan 20, 2022 · 5 comments · Fixed by #7921
Closed

Cabal run leaves children running when killed #7914

robx opened this issue Jan 20, 2022 · 5 comments · Fixed by #7921

Comments

@robx
Copy link
Collaborator

robx commented Jan 20, 2022

Describe the bug

When cabal run receives SIGTERM, it exits, but leaves its children running.

I'm not entirely sure whether this is buggy behaviour, but I was surprised by this,
in the context of running a Haskell tool via cabal run and withProcessTerm.

If I run cabal run target from the shell, and run
kill <cabal-run-pid>, I would have expected both cabal and target
to stop. Instead, cabal exits but target keeps running.

Note that when run interactively, Ctrl-C sends SIGINT to the whole
process group, which is how cabal run interrupts nicely on Ctrl-C.

To Reproduce

$ cat x.hs
{- cabal:
build-depends: base
-}

module Main where

import Control.Concurrent (threadDelay)
import Control.Exception
import Control.Monad (when)

main = loop `catch` logInterrupt
  where
    loop = do
      putStrLn "yo"
      threadDelay 1000000
      loop
    logInterrupt e = do
      when (e == UserInterrupt) $ putStrLn "ouch"
      throwIO e

Then call cabal run x.hs, in a different terminal:

$ ps | grep cabal
77565 ttys002    0:04.09 cabal run x.hs
77726 ttys002    0:00.01 /s/cabal/dist-newstyle/build/x86_64-osx/ghc-8.10.7/fake-package-0/x/script/build/script/script
$ kill 77565

Then the child keeps running although cabal exits:

yo
yo
yo
yo
Terminated: 15
$ yo
yo
yo

Expected behavior

I'd like the child process to be killed, too.

System information

  • Operating system: macos
  • cabal-install version 3.4.0.0, compiled using version 3.4.0.0 of the Cabal library
  • ghc 8.10.7
@Mikolaj
Copy link
Member

Mikolaj commented Jan 20, 2022

I seem to remember an old ticket about that.

@robx
Copy link
Collaborator Author

robx commented Jan 20, 2022

I think in some sense the right/expected thing would be for cabal run to exec() instead of forking.

@Mikolaj
Copy link
Member

Mikolaj commented Jan 20, 2022

Oh, yes, I know some of these words. The related issue might have been one of these: #7757 or #7309.

@robx
Copy link
Collaborator Author

robx commented Jan 20, 2022

Ah, yes, #7757 seems to address this issue explicitly. Thanks! I'd looked through issues but not PRs.

@robx
Copy link
Collaborator Author

robx commented Jan 24, 2022

I'm convinced this is a bug by now. cabal should stop and wait for all its children when killed.

As a work-around, I'm using this shell script to replace cabal run:

#!/usr/bin/env bash

#   cabal-run.sh target args
#
# is equivalent to
#
#   cabal run target -- args
#
# except that killing it will also kill the target.
#
# https://github.com/haskell/cabal/issues/7914

set -euo pipefail

target="$1"
shift

echo "building target: ${target}"
cabal build "$target"

executable=$(cabal list-bin "$target")

exec "$executable" "$@"

hasura-bot pushed a commit to hasura/graphql-engine that referenced this issue Jan 24, 2022
There's two major parts to this change:

1. cut down on environment variables needed to run the tests:
  - lux parameters that don't change are now in code
  - most remaining parameters have reasonable defaults
  - the only variable that is still required to be set is HASURA_MULTITENANT_SPEC_DB_URL
  - env.sh is no longer needed
2. find a better work-around for the problems running graphql-engine-multitenant
  via cabal run (haskell/cabal#7914), by adding a shell
  script that implements a more correct version of cabal run.

PR-URL: hasura/graphql-engine-mono#3461
GitOrigin-RevId: 0939a79cc45cd3c1c103719552b12099678850dd
@mergify mergify bot closed this as completed in #7921 Mar 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants