Skip to content
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

Process#stop does not kill process tree #66

Closed
jarib opened this issue Jan 30, 2014 · 3 comments
Closed

Process#stop does not kill process tree #66

jarib opened this issue Jan 30, 2014 · 3 comments

Comments

@jarib
Copy link
Collaborator

jarib commented Jan 30, 2014

require 'childprocess'

File.open("test.bat", "w") do |io|
  io << '@ruby -e sleep'
end

proc = ChildProcess.build("test.bat")
proc.io.inherit!
proc.start

sleep 3

proc.stop

puts `tasklist`.split("\n").grep(/ruby/)
@jarib
Copy link
Collaborator Author

jarib commented Jan 30, 2014

But should it? Doesn't happen on Unix either:

require 'childprocess'
require 'fileutils'

File.open("test.sh", "w") do |io|
  io << "#/bin/bash\nruby -e sleep"
end

FileUtils.chmod 0755, 'test.sh'

proc = ChildProcess.build('./test.sh')
proc.io.inherit!
proc.start

sleep 3

proc.stop
puts `ps auxww | grep sleep`

@jarib
Copy link
Collaborator Author

jarib commented Jan 30, 2014

That was simple enough to solve for Unix (now in the kill-process-tree branch), but not sure it's going to be this easy on Windows or Unix+JRuby.

@jarib
Copy link
Collaborator Author

jarib commented Jan 31, 2014

This should be fixed now for all platforms, and I've pushed v4.1.0.rc1 which includes the changes.

The only problem is JRuby + ProcessBuilder, which is currently ChildProcess' weapon of choice on Unix. However, it looks like the latest JRuby (at least 1.7.9) now works well with posix_spawn, so JRuby users have the option to enable that with ChildProcess.posix_spawn = true. We should probably make that the default at some point.

I'm also not completely confident about the implications of this change, so will do some more testing before pushing a real release.

@jarib jarib closed this as completed Jan 31, 2014
jarib added a commit that referenced this issue Feb 17, 2014
This closes issue #69 and #71. However, it re-introduces the problems
from #66, which can be worked around like so:

  process = ChildProcess.build(...)
  process.leader = true
  process.start
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

1 participant