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

Replace all backends by Process.spawn for portability, reliability and simplicity #175

Merged
merged 4 commits into from
Jan 7, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: CI
on: [push, pull_request]
jobs:
test:
strategy:
fail-fast: false
matrix:
os: [ ubuntu, macos, windows ]
ruby: [ '2.4', '2.5', '2.6', '2.7', '3.0', '3.1', '3.2', '3.3', jruby, truffleruby ]
exclude:
- { 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
steps:
- uses: actions/checkout@v2
- uses: ruby/setup-ruby@v1
with:
ruby-version: ${{ matrix.ruby }}
bundler-cache: true
- run: bundle exec rake spec
37 changes: 0 additions & 37 deletions .travis.yml

This file was deleted.

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: 3 additions & 20 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,6 @@ a standalone library.

* Ruby 2.4+, JRuby 9+

Windows users **must** ensure the `ffi` gem (`>= 1.0.11`) is installed in order to use ChildProcess.

# Usage

The object returned from `ChildProcess.build` will implement `ChildProcess::AbstractProcess`.
Expand Down Expand Up @@ -73,9 +71,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 +136,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 Down Expand Up @@ -195,11 +182,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
36 changes: 0 additions & 36 deletions appveyor.yml

This file was deleted.

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
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
Loading