Skip to content

cabal-install doesn't respect ^C #6322

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

Closed
phadej opened this issue Oct 31, 2019 · 7 comments
Closed

cabal-install doesn't respect ^C #6322

phadej opened this issue Oct 31, 2019 · 7 comments

Comments

@phadej
Copy link
Collaborator

phadej commented Oct 31, 2019

Describe the bug

One have to ^C a lot to make cabal cancel. This is super annoying

To Reproduce

cabal v2-build something with plenty (fresh to build) dependencies. Try to ^C build right away. It won't. (on Linux).

Expected behavior

cabal should cancel promptly.

System information

  • Operating System: Linux
  • cabal-install-3.0.0.0

Additional context

E.g. when built through make, make seems to forward ^C, and then dies itself, leaving "downloading" cabal on the background. That's bad.

Feels like some missing bracket somewhere.

@phadej
Copy link
Collaborator Author

phadej commented Oct 31, 2019

^Cghc-pkg: interrupted
gi^Cghc-pkg: interrupted
^Cghc-pkg: interrupted
^Cghc-pkg: interrupted
^Cghc-pkg: interrupted
^C^C^Cghc-pkg: interrupted

@23Skidoo
Copy link
Member

23Skidoo commented Feb 7, 2020

I remember that we already had some code/tickets related to this, but don't remember where. @dcoutts?

robx added a commit to robx/cabal that referenced this issue Jan 25, 2022
…6322)

The concrete bug is that `try` catches asynchronous exceptions,
preventing `withAsync` from interrupting it when its `body`
action finishes prematurely.

This manifests during `cabal build` when hitting Ctrl-C during
download, e.g.:

1. Ctrl-C interrupts a curl process
2. This interrupt is converted into a UserInterrupt exception
   via the process package's delegate_ctlc functionality.
3. UserInterrupt bubbles up through `fetchPackage`, is caught by
   the `try` (fine) and writen to the result mvar.
   Meanwhile, `fetchPackage` continues its loop, starting to
   fetch the next package.
4. Some `rebuildTarget` is waiting for the interrupted download;
  `waitAsyncFetchPackage` rethrows UserInterrupt in that thread.
5. UserInterrupt bubbles up through `collectJob`, `execute`, the
   `body` action.
6. `withAsync`'s finalizer cancels the `fetchPackages` async.
7. The `fetchPackages` async receives the AsyncCancelled exception
   while fetching the next package (see 3. above). This interrupts
   the download action (it shouldn't matter whether we happen to
   be running curl again or not), and AsyncCancelled bubbles up
   through `fetchPackage`, is caught by the `try` (not fine!) and
   written to the mvar. But no-one is reading that mvar anymore
   because the build job already aborted (step 5), and that
   AsyncCancelled exception is lost.
8. `fetchPackages` keeps doing its thing, exiting eventually.
robx added a commit to robx/cabal that referenced this issue Jan 25, 2022
…6322)

The concrete bug is that `try` catches asynchronous exceptions,
preventing `withAsync` from interrupting it when its `body`
action finishes prematurely.

This manifests during `cabal build` when hitting Ctrl-C during
download, e.g.:

1. Ctrl-C interrupts a curl process
2. This interrupt is converted into a UserInterrupt exception
   via the process package's delegate_ctlc functionality.
3. UserInterrupt bubbles up through `fetchPackage`, is caught by
   the `try` (fine) and writen to the result mvar.
   Meanwhile, `fetchPackage` continues its loop, starting to
   fetch the next package.
4. Some `rebuildTarget` is waiting for the interrupted download;
  `waitAsyncFetchPackage` rethrows UserInterrupt in that thread.
5. UserInterrupt bubbles up through `collectJob`, `execute`, the
   `body` action.
6. `withAsync`'s finalizer cancels the `fetchPackages` async.
7. The `fetchPackages` async receives the AsyncCancelled exception
   while fetching the next package (see 3. above). This interrupts
   the download action (it shouldn't matter whether we happen to
   be running curl again or not), and AsyncCancelled bubbles up
   through `fetchPackage`, is caught by the `try` (not fine!) and
   written to the mvar. But no-one is reading that mvar anymore
   because the build job already aborted (step 5), and that
   AsyncCancelled exception is lost.
8. `fetchPackages` keeps doing its thing, exiting eventually.

Note that this change affects both instances of `try` in this story:
`UserInterrupt` is also an asynchronous exception, so after the
change the `UserInterrupt` takes a different path from step 3. That
appears to be fine, though.
@robx
Copy link
Collaborator

robx commented Jan 26, 2022

I might have a fix for this in #7929, but it's a bit unclear if there's a different bug. I see the described behaviour, but only when there are (multiple) packages to be downloaded, and I try to interrupt during download. I've managed to reproduce that bug, and fix it in the PR. From the description and terminal log here it's unclear whether downloading was involved.

@ulysses4ever
Copy link
Collaborator

@robx I think we should close it, do you agree? I just tried to Ctrl-C at various stages while building lens on Linux. It worked like a charm.

@robx
Copy link
Collaborator

robx commented Feb 28, 2023

Yes, that sounds good to me -- this report is Linux-specific, and to the best of my knowledge the issue is fixed there. (I think the fix didn't make it into 3.8, but should be there in current master and 3.10.)

I'll close, of course feel free to reopen this if it resurfaces, or if we should wait for the 3.10 release before closing the issue.

@robx robx closed this as completed Feb 28, 2023
@ulysses4ever
Copy link
Collaborator

I was trying on 3.8 by the way. And #7921 definitely got in 3.8.

@robx
Copy link
Collaborator

robx commented Feb 28, 2023

I was trying on 3.8 by the way. And #7921 definitely got in 3.8.

I think it's #7929 that fixed this, which did make it into 3.8.

(The primary effect of #7921 is to ensure that killing cabal run kills whatever cabal run is running as well as cabal itself.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants