ghcid icon indicating copy to clipboard operation
ghcid copied to clipboard

Suggestion to fix dangling ghc process on double ctrl+c

Open asterite opened this issue 6 years ago • 14 comments

Hi!

This PR is not meant to be merged. Instead, I'd like to open the discussion to find possible solutions to #257

Debugging the code, not fully looking at it, I noticed that when you press ctrl+c once it seems quit will be called, which in turn will call interrupt and then ghciInterrupt. However, if you press ctrl+c once again the process will shutdown immediately and a dangling ghc might happen.

Just to try things to improve this, I used installHandler to trap SIGINT (ctrl+c). When I did that, every time you press ctrl+c, no matter how many times, the handler call will run. So I tried putting quit there and it works fine. However, the second time it will print some failure (because we already called terminateProcess) but it at least doesn't leave a dangling ghc, so it's already slightly better than the current situation. Ideally the second ctrl+c would do nothing: just wait for the process to exit, or maybe just ignore the second ctrl+c, I don't know.

Thoughts?

The dangling ghc is not a blocker but I'd definitely like to prevent it.

asterite avatar Nov 04 '19 15:11 asterite

Aren't you trading a process that leaks a GHC, for one you can't kill with Ctrl-C at all? Both seem like fairly unpleasant options...

The other issue is that ghcid is both a library and an executable, but I'm sure with some config we could work around that.

ndmitchell avatar Nov 06 '19 23:11 ndmitchell

Aren't you trading a process that leaks a GHC, for one you can't kill with Ctrl-C at all?

It does get killed. You just get an error message when you press that second ctrl+c, but other than that it works well.

but I'm sure with some config we could work around that.

Yes! An option to make this configurable sounds great.

asterite avatar Nov 06 '19 23:11 asterite

It does get killed. You just get an error message when you press that second ctrl+c, but other than that it works well.

I'm confused - can you explain the sequence of events? I hit ctrl-c, that starts terminating GHC how it does now. I hit ctrl-c again, then I go to the handler, and what happens? What if GHC exits cleanly, and what if it stays around? If I hit ctrl-c again, what happens then? (I'm a Windows guy, so signal handlers aren't my area of expertise!)

ndmitchell avatar Nov 06 '19 23:11 ndmitchell

Oh, I don't know about signals either, much less in Haskell. This was just an experiment to see ig I could kill the underlying process no matter how many ctrl+c I did.

I'll investigate this with my team to see if we can come to a clean PR that works as expected.

Thank you!

asterite avatar Nov 06 '19 23:11 asterite

Happy to keep this PR open while you do so (up to you) - I agree this seems like a reasonable approach. I'd also suggest you stick to Posix only to start with, since doing whatever you do cross platform will be additional complication. (Although very happy if you can do it cross platform, of course!)

ndmitchell avatar Nov 06 '19 23:11 ndmitchell

Sounds good!

asterite avatar Nov 07 '19 00:11 asterite

Hello,

I've experienced this issue recently, and I think that it would be very useful to fix it 🙂

Basically my computer was becoming unresponsive after using ghcid with a very heavy project. I realized ghci processes would start to become daemonic and consume lots of memory and CPU. Apparently ghcid was not able to stop them, so it would leave them hanging around. This is quite confusing, since as a user I would expect to ignore that there's an underlying ghci process, and clearly it should not be left running in the system without any notice.

I have a draft commit with a fix https://github.com/ndmitchell/ghcid/commit/9f1eb0fe622c0d95955760ddd0590fd73166a2f9 . My proposal is: don't exit ghcid unless ghci is dead. This means that a user may need to manually kill ghcid in case ghci hangs; but at least, user will be aware that there's something wrong with current state, and check what action needs to be taken.

Some notes about my commit:

  • Note that the Haskell runtime will simply kill the current app when receiving a second SIGINT, leaving a daemonic ghci running in all cases
  • I think we should ignore the second SIGINT altogether: the first SIGINT will try and stop ghci, if it doesn't die, then there's nothing else we can do. First call to installHandler does that.
  • The function I patched (Session.kill) is also used by ghcid to kill an unresponsive ghci and start a new one. So we need to be careful to restore the original signal handler in case this kill action does not originate from a SIGINT. Second call to installHandler does that.

acondolu avatar Mar 04 '22 14:03 acondolu

@acondolu your solution sounds pretty reasonable to me, and has the nice advantage of being cross platform. Thoughts @asterite? A second set of eyes on such pieces is always useful.

ndmitchell avatar Mar 26 '22 12:03 ndmitchell

Notes if we do go with @acondolu's code:

  • Would be better to use a bracket for disabling the interrupt, so we don't forget to add it back.
  • There's a lot of good comments in this thread and the message that would be useful to move the knowledge into the code as comments.

ndmitchell avatar Mar 26 '22 12:03 ndmitchell

@ndmitchell I've revisited my commit with your observations ➡️ see https://github.com/acondolu/ghcid/commit/d299f9a4cbdf7a2278d7c5b1be03adfeb4cc195b (now using bracket + added a comment)

What do you guys think?

acondolu avatar Apr 01 '22 16:04 acondolu

Any chance to get this going?

tchoutri avatar May 12 '22 19:05 tchoutri

@asterite Sorry for giving no thoughts about this, but a lot of time has passed and I lost context on this, plus I won't have time to look at it... so don't wait for my opinion 😄 (in case you were doing so)

asterite avatar May 12 '22 20:05 asterite

@acondolu does that mean if ghci doesn't exit, ghcid hangs?

domenkozar avatar Jan 14 '24 14:01 domenkozar

Current state: pressing Ctrl+C twice will make ghcid exit but the underlying ghci dangling, running in the background.

Proposed state: after pressing Ctrl+C (once or multiple times) if ghci doesn't exit, ghcid hangs.

acondolu avatar Jan 15 '24 18:01 acondolu