Skip to content

Remove fork usage for JRuby compatibility #1002

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.

Sign up for GitHub

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

Draft
myronmarston wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Remove fork usage for JRuby compatibility #1002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
78 changes: 39 additions & 39 deletions elasticgraph-local/lib/elastic_graph/local/docker_runner.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,15 @@ def initialize(variant, port:, ui_port:, version:, env:, ready_log_line:, daemon
@output = output
end

# :nocov: -- difficult to test `exec` behavior (replaces current process)
def boot
# :nocov: -- this is actually covered via a call from `boot_as_daemon` but it happens in a forked process so simplecov doesn't see it.
halt

prepare_docker_compose_run "up" do |command|
exec(command) # we use `exec` so that our process is replaced with that one.
end
# :nocov:
end
# :nocov:

def halt
prepare_docker_compose_run "down --volumes" do |command|
Expand All @@ -40,26 +40,11 @@ def halt
end

def boot_as_daemon(halt_command:)
with_pipe do |read_io, write_io|
fork do
# :nocov: -- simplecov can't track coverage that happens in another process
read_io.close
Process.daemon
pid = Process.pid
$stdout.reopen(write_io)
$stderr.reopen(write_io)
puts pid
boot
write_io.close
# :nocov:
end

# The `Process.daemon` call in the subprocess changes the pid so we have to capture it this way instead of using
# the return value of `fork`.
pid = read_io.gets.to_i
halt

@output.puts "Booting #{@variant}; monitoring logs for readiness..."
@output.puts "Booting #{@variant}; monitoring logs for readiness..."

pid = spawn_docker_compose_up do |read_io|
::Timeout.timeout(
@daemon_timeout,
::Timeout::Error,
Expand All @@ -76,20 +61,46 @@ def boot_as_daemon(halt_command:)
break if @ready_log_line.match?(line.to_s)
end
end
end

@output.puts
@output.puts
@output.puts <<~EOS
Success! #{@variant} #{@version} (pid: #{pid}) has been booted for the #{@env} environment on port #{@port}.
It will continue to run in the background as a daemon. To halt it, run:
# Detach so the process continues running after this Ruby process exits.
::Process.detach(pid)

#{halt_command}
EOS
end
@output.puts
@output.puts
@output.puts <<~EOS
Success! #{@variant} #{@version} (pid: #{pid}) has been booted for the #{@env} environment on port #{@port}.
It will continue to run in the background as a daemon. To halt it, run:

#{halt_command}
EOS
end

private

def spawn_docker_compose_up
read_io, write_io = ::IO.pipe

pid = prepare_docker_compose_run("up") do |command|
spawn(
command,
chdir: ::Dir.pwd,
out: write_io,
err: write_io
)
end

write_io.close # We don't write from the parent process

begin
yield read_io
ensure
read_io.close
end

pid
end

def prepare_docker_compose_run(*commands)
name = "#{@env}-#{@version.tr(".", "_")}"

Expand All @@ -101,17 +112,6 @@ def prepare_docker_compose_run(*commands)
yield full_command
end
end

def with_pipe
read_io, write_io = ::IO.pipe

begin
yield read_io, write_io
ensure
read_io.close
write_io.close
end
end
end
end
end
5 changes: 3 additions & 2 deletions elasticgraph-local/lib/elastic_graph/local/rake_tasks.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -475,9 +475,10 @@ def define_other_tasks

# :nocov: -- we can't test `open` behavior through a test
unless args.fetch(:no_open)
fork do
Thread.new do
sleep 3 # give the app a bit of time to boot before we try to open it.
sh "open http://localhost:#{port}/"
url = "http://localhost:#{port}/"
system("open", url) || system("xdg-open", url)
end
end
# :nocov:
Expand Down
4 changes: 2 additions & 2 deletions elasticgraph-local/sig/elastic_graph/local/docker_runner.rbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ module ElasticGraph

private

def prepare_docker_compose_run: (*::String) { (::String) -> void } -> void
def with_pipe: [T] () { (::IO, ::IO) -> T } -> T
def spawn_docker_compose_up: () { (::IO) -> void } -> ::Integer
def prepare_docker_compose_run: [T] (*::String) { (::String) -> T } -> T
end
end
end
6 changes: 6 additions & 0 deletions spec_support/lib/elastic_graph/spec_support/in_sub_process.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,13 @@ module ElasticGraph
# Runs the provided block in a subprocess. Any failures in the sub process get
# caught and re-raised in the parent process. Also, this returns the return value
# of the child process (using `Marshal` to send it across a pipe).
#
# On JRuby, fork is unavailable, so this skips the test instead.
def in_sub_process(&block)
# :nocov: -- we only cover one side of this conditional for a given run of the test suite
skip "Test requires fork (unavailable on JRuby)" if RUBY_ENGINE == "jruby"
# :nocov:

SubProcess.new.run(&block)
end
end
Expand Down