Suspend and Resume on TSTP and CONT signals #361

[Kache]

ctrl-z (SIGTSTP) doesn't work with Spring, and it corrupts the terminal

as mentioned by @casper in Issue #361:

the spring server is somehow attached to the PTY of the shell

the solution is to:

trap SIGTSTP and tell the server to disengage from the PTY
before the client process is suspended

[rails-bot]

Thanks for the pull request, and welcome! The Rails team is excited to review your changes, and you should hear from @jonleighton (or someone else) soon.

If any changes to this PR are deemed necessary, please add them as extra commits. This ensures that the reviewer can see what has changed since they last reviewed the code. Due to the way GitHub handles out-of-date commits, this should also make it reasonably obvious what issues have or haven't been addressed. Large or tricky changes may require several passes of review and changes.

Please see the contribution instructions for more information.

[jonleighton]

Is there any way we can write a test for this?

[Kache]

I tried to find how the current signal handlers are being tested, and I wasn't able to find any existing tests for them, e.g. Spring::Client::Run::FORWARDED_SIGNALS.

Are they someplace else? I don't have enough familiarity with how spring works to write tests for the server/client signal handling from scratch.

[jonleighton]

Yeah OK. I had thought there was some sort of similar test already, but I see that there's not. I agree it could be quite tricky to write a test for this. When I get a chance I'll do some manual testing and then merge your code.

[TylerRick]

Any word on this?

It's a really confusing experience when you Ctrl-Z and can't even get back because half of your keystrokes seem to go to your shell and half to ruby.

Fortunately it's easy to manually patch $(bundle show spring)/lib/spring/client/run.rb for now. 😄

[Kache]

@jonleighton It's been a long time since this simple PR was first submitted. Can you take a look? I've rebased it on top of master without any additional changes.

By the way, there's some nondeterminism in tests. For the same code:

• passed: https://travis-ci.org/rails/spring/builds/427907936
• failed: https://travis-ci.org/rails/spring/builds/427901502

[JoshCheek]

I came to the same conclusion about this just recently and had basically the same solution except I didn't realize the difference between TSTP and STOP, so my trap registration was a lot more convoluted: https://gist.github.com/JoshCheek/c34d54e87fc771d2972e50c9900cfaeb

Here's a gif showing the core ideas, though it's of my demo and not this PR:

[jeremy]

That's cool @JoshCheek

[JoshCheek]

Thanks for merging! Excited to have this available ^_^