Skip to content

Commit

Permalink
Use Process.spawn, it is enough to implement ChildProcess
Browse files Browse the repository at this point in the history
* Process.spawn requires only very minimal variants between platforms.
* Remove all other backends, Process.spawn is enough.
  • Loading branch information
eregon committed Jan 4, 2024
1 parent 53f811f commit 228ec6d
Show file tree
Hide file tree
Showing 35 changed files with 254 additions and 2,114 deletions.
6 changes: 4 additions & 2 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ jobs:
fail-fast: false
matrix:
os: [ ubuntu, macos, windows ]
ruby: [ 2.6, jruby, truffleruby ]
ruby: [ '2.4', '2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', jruby, truffleruby ]
exclude:
- { os: windows, ruby: truffleruby }
- { os: windows, ruby: truffleruby }
# fails to load rspec: RuntimeError: CRITICAL: RUBYGEMS_ACTIVATION_MONITOR.owned?: before false -> after true
- { os: windows, ruby: jruby }
runs-on: ${{ matrix.os }}-latest
env:
CHILDPROCESS_UNSET: should-be-unset
Expand Down
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
### Version 5.0.0 / TODO

* [#175](https://github.com/enkessler/childprocess/pull/175): Replace all backends by `Process.spawn` for portability, reliability and simplicity.

### Version 4.1.0 / 2021-06-08

* [#170](https://github.com/enkessler/childprocess/pull/170): Update gem homepage to use `https://`
Expand Down
2 changes: 0 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,3 @@ gemspec

# Used for local development/testing only
gem 'rake'

gem 'ffi' if ENV['CHILDPROCESS_POSIX_SPAWN'] == 'true' || Gem.win_platform?
23 changes: 5 additions & 18 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,9 @@ begin
process = ChildProcess.build("sh" , "-c",
"for i in {1..3}; do echo $i; sleep 1; done")
process.io.stdout = w
process.start # This results in a fork, inheriting the write end of the pipe.
process.start # This results in a subprocess inheriting the write end of the pipe.

# Close parent's copy of the write end of the pipe so when the (forked) child
# Close parent's copy of the write end of the pipe so when the child
# process closes its write end of the pipe the parent receives EOF when
# attempting to read from it. If the parent leaves its write end open, it
# will not detect EOF.
Expand Down Expand Up @@ -138,17 +138,6 @@ search.io.stdin.close
search.wait
```

#### Prefer posix_spawn on *nix

If the parent process is using a lot of memory, `fork+exec` can be very expensive. The `posix_spawn()` API removes this overhead.

```ruby
ChildProcess.posix_spawn = true
process = ChildProcess.build(*args)
```

To be able to use this, please make sure that you have the `ffi` gem installed.

### Ensure entire process tree dies

By default, the child process does not create a new process group. This means there's no guarantee that the entire process tree will die when the child process is killed. To solve this:
Expand All @@ -159,6 +148,8 @@ process.leader = true
process.start
```

Note this does not work on Windows.

#### Detach from parent

```ruby
Expand Down Expand Up @@ -195,11 +186,7 @@ ChildProcess.logger = logger

# Implementation

How the process is launched and killed depends on the platform:

* Unix : `fork + exec` (or `posix_spawn` if enabled)
* Windows : `CreateProcess()` and friends
* JRuby : `java.lang.{Process,ProcessBuilder}`
ChildProcess 5+ uses `Process.spawn` from the Ruby core library for maximum portability.

# Note on Patches/Pull Requests

Expand Down
63 changes: 11 additions & 52 deletions lib/childprocess.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
require 'childprocess/errors'
require 'childprocess/abstract_process'
require 'childprocess/abstract_io'
require 'childprocess/process_spawn_process'
require "fcntl"
require 'logger'

Expand All @@ -15,13 +16,7 @@ class << self
def new(*args)
case os
when :macosx, :linux, :solaris, :bsd, :cygwin, :aix
if jruby? && !posix_spawn_chosen_explicitly?
JRuby::Process.new(args)
elsif posix_spawn?
Unix::PosixSpawnProcess.new(args)
else
Unix::ForkExecProcess.new(*args)
end
Unix::Process.new(*args)
when :windows
Windows::Process.new(*args)
else
Expand All @@ -40,13 +35,7 @@ def logger
end

def platform
if RUBY_PLATFORM == "java"
:jruby
elsif defined?(RUBY_ENGINE) && RUBY_ENGINE == "ironruby"
:ironruby
else
os
end
os
end

def platform_name
Expand All @@ -62,7 +51,7 @@ def linux?
end

def jruby?
platform == :jruby
RUBY_ENGINE == 'jruby'
end

def windows?
Expand All @@ -74,27 +63,6 @@ def posix_spawn_chosen_explicitly?
end

def posix_spawn?
enabled = posix_spawn_chosen_explicitly? || !Process.respond_to?(:fork)
return false unless enabled

begin
require 'ffi'
rescue LoadError
raise ChildProcess::MissingFFIError
end

begin
require "childprocess/unix/platform/#{ChildProcess.platform_name}"
rescue LoadError
raise ChildProcess::MissingPlatformError
end

require "childprocess/unix/lib"
require 'childprocess/unix/posix_spawn_process'

true
rescue ChildProcess::MissingPlatformError => ex
warn_once ex.message
false
end

Expand All @@ -107,6 +75,8 @@ def posix_spawn=(bool)
end

def os
return :windows if ENV['FAKE_WINDOWS'] == 'true'

@os ||= (
require "rbconfig"
host_os = RbConfig::CONFIG['host_os'].downcase
Expand Down Expand Up @@ -162,17 +132,6 @@ def arch
def close_on_exec(file)
if file.respond_to?(:close_on_exec=)
file.close_on_exec = true
elsif file.respond_to?(:fcntl) && defined?(Fcntl::FD_CLOEXEC)
file.fcntl Fcntl::F_SETFD, Fcntl::FD_CLOEXEC

if jruby? && posix_spawn?
# on JRuby, the fcntl call above apparently isn't enough when
# we're launching the process through posix_spawn.
fileno = JRuby.posix_fileno_for(file)
Unix::Lib.fcntl fileno, Fcntl::F_SETFD, Fcntl::FD_CLOEXEC
end
elsif windows?
Windows::Lib.dont_inherit file
else
raise Error, "not sure how to set close-on-exec for #{file.inspect} on #{platform_name.inspect}"
end
Expand Down Expand Up @@ -207,8 +166,8 @@ def is_64_bit?
end # class << self
end # ChildProcess

require 'jruby' if ChildProcess.jruby?

require 'childprocess/unix' if ChildProcess.unix?
require 'childprocess/windows' if ChildProcess.windows?
require 'childprocess/jruby' if ChildProcess.jruby?
if ChildProcess.windows?
require 'childprocess/windows'
else
require 'childprocess/unix'
end
4 changes: 3 additions & 1 deletion lib/childprocess/abstract_process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ class AbstractProcess
# Set this to true to make the child process the leader of a new process group
#
# This can be used to make sure that all grandchildren are killed
# when the child process dies.
# when the child process dies on non-Windows.
#
# Note that grandchildren are not killed on Windows even when the process is the leader of a new process group.
#
attr_accessor :leader

Expand Down
21 changes: 0 additions & 21 deletions lib/childprocess/errors.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,25 +13,4 @@ class InvalidEnvironmentVariable < Error

class LaunchError < Error
end

class MissingFFIError < Error
def initialize
message = "FFI is a required pre-requisite for Windows or posix_spawn support in the ChildProcess gem. " +
"Ensure the `ffi` gem is installed. " +
"If you believe this is an error, please file a bug at http://github.com/enkessler/childprocess/issues"

super(message)
end

end

class MissingPlatformError < Error
def initialize
message = "posix_spawn is not yet supported on #{ChildProcess.platform_name} (#{RUBY_PLATFORM}), falling back to default implementation. " +
"If you believe this is an error, please file a bug at http://github.com/enkessler/childprocess/issues"

super(message)
end

end
end
56 changes: 0 additions & 56 deletions lib/childprocess/jruby.rb

This file was deleted.

16 changes: 0 additions & 16 deletions lib/childprocess/jruby/io.rb

This file was deleted.

Loading

0 comments on commit 228ec6d

Please sign in to comment.