Suggestion to fix dangling ghc process on double ctrl+c

[asterite]

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.

[ndmitchell]

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.

[asterite]

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.

[ndmitchell]

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!)

[asterite]

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!

[ndmitchell]

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!)

[asterite]

Sounds good!

[acondolu]

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 9f1eb0f . 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.

[ndmitchell]

@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]

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.

[acondolu]

@ndmitchell I've revisited my commit with your observations ➡️ see acondolu@d299f9a
(now using bracket + added a comment)

What do you guys think?

[tchoutri]

Any chance to get this going?

[asterite]

@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)

[domenkozar]

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

[acondolu]

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.