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

Fix ChildProcess::Unix::Process#exited? method with leader processes #154

Merged
merged 1 commit into from
Jul 7, 2019

Conversation

jdelStrother
Copy link
Contributor

Sorry, I found another edge case following on from my leader-process problems in #151

require "childprocess"

ChildProcess.logger.level = 0

# Start a background thread that launches a long-lived process.
# The child process will be killed once this script finishes (via at_exit)
Thread.new do
  child1 = ChildProcess.build("yes")
  child1.start
  at_exit do
    child1.stop
  end
  status = child1.wait
end

# give the bg thread some time to ensure child1.wait has happened
sleep 0.1

# Start a second child process
child2 = ChildProcess.build("sleep", "100")
child2.leader = true
child2.start
puts "child2.stop"
child2.stop
puts "Child2 is done!"

When child2.stop is called, we send it TERM, then poll for it to exit with ::Process.waitpid2(_pid, ::Process::WNOHANG | ::Process::WUNTRACED). The process dies straight away, but under ruby 2.6, that waitpid2 call always returns nil, until we hit the stop-timeout, at which point we sent the child a KILL.

In OS X, that KILL raises Errno::EPERM. In Linux, it seems like it just swallows the signal being sent to a dead process.

WDYT to this fix? I'm not really familiar with unix leader process, so would welcome additional scrutiny.

After sending TERM to a leader process, `::Process.waitpid2(_pid, ::Process::WNOHANG | ::Process::WUNTRACED)` appears to just return nil under ruby 2.6.

This causes process.stop to send TERM to the process, poll (unsuccessfully) for the exit status until the timeout is hit, then send KILL.
@coveralls
Copy link

Coverage Status

Coverage remained the same at 93.715% when pulling 5d1096d on jdelStrother:patch-1 into 3bbc30f on enkessler:dev.

Copy link
Collaborator

@sds sds left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the PR.

Supportive of this solution given the documentation for waitpid(2):

wait for any child process whose process group ID is equal to the absolute value of pid.

In this context, we don't want to wait for any process in the process group—we are only interested in the process itself, so we should simply wait on the lone process, not any member of the group.

@sds sds merged commit 3912527 into enkessler:dev Jul 7, 2019
@sds sds added the bug label Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants