Kill PTY I/O thread when we are done with it #497
Conversation
|
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. |
misterbyrne
force-pushed
the
ab/kill-thread
branch
2 times, most recently
from
e5349c9 to
bf363c3
Compare
Sometimes the PTY slave can be closed, causing an unhandled exception in the thread which is calling `master.read` (socket closed). This change kills the thread before this happens. Related to rails#396 (since this exception was causing Spring to hang)
| @@ -295,10 +295,11 @@ def print_exception(stream, error) | |||
| def with_pty | |||
| PTY.open do |master, slave| | |||
| [STDOUT, STDERR, STDIN].each { |s| s.reopen slave } | |||
| Thread.new { master.read } | |||
| reader_thread = Spring.failsafe_thread { master.read } | |||
There was a problem hiding this comment.
I've switched to using Spring.failsafe_thread here for consistency
|
@jonleighton Does this look okay to you? |
|
Sorry, I haven't got round to reviewing it yet, but it's on my list and hasn't been forgotten. |
|
@jonleighton could you please release a new version to rubygems? i haven't had any issues with spring hanging since using this version |
|
@mikesea done |
Sometimes the PTY slave can be closed, causing an unhandled exception in
the thread which is calling
master.read(socket closed).This change kills the thread before this happens.
This solves #396 for the test case included
(since this exception was causing Spring to hang)