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

Raising SIGTERM SignalException returns wrong process exit code #5134

Closed
perlun opened this Issue Apr 11, 2018 · 5 comments

Comments

Projects
None yet
2 participants
@perlun
Contributor

perlun commented Apr 11, 2018

(This was originally spotted in puma/puma#1558, thanks @dekellum for great help there.)

With JRuby 9.1.16.0 on Java 8 (on latest macOS), I get this:

$ jruby -e 'raise SignalException, "SIGTERM"' ; echo $?
SignalException: SIGTERM
  <main> at -e:1
1

Same with JRuby 1.7 actually:

$ rvm use jruby-1.7.27
[09:59:19] plundberg@ecvaawplun6:~/git/tmp (master)
$ jruby -e 'raise SignalException, "SIGTERM"' ; echo $?
SignalException: SIGTERM
  (root) at -e:1
1

However, with MRI 2.5.1 I get this:

$ ruby -e 'raise SignalException, "SIGTERM"' ; echo $?
Terminated: 15
143

...which is indeed the correct value for a SIGTERM-triggered exit of a process.


As inspired by #5049, I tried another approach also which worked slightly differently:

$ rvm use 2.5.1
Using /Users/plundberg/.rvm/gems/ruby-2.5.1
$ ruby -e "Process.kill('TERM', Process.pid)" ; echo $?
Terminated: 15
143

Here be the surprise: JRuby behaves differently, and even differently than the raise SignalException, "SIGTERM" scenario.

$ rvm use jruby
Using /Users/plundberg/.rvm/gems/jruby-9.1.16.0
$ ruby -e "Process.kill('TERM', Process.pid)" ; echo $?
0

Exit code 0 is not entirely correct, but it's at least much better than exit code 1 which indicates failure. 😄

Inspired by this Oracle article linked to from the JRuby wiki, I tested with the -Xrs parameter:

$ rvm use jruby
Using /Users/plundberg/.rvm/gems/jruby-9.1.16.0
$ export JAVA_OPTS=-Xrs
$ ruby -e "Process.kill('TERM', Process.pid)" ; echo $?
Terminated: 15
143
$ ruby -e 'raise SignalException, "SIGTERM"' ; echo $?
SignalException: SIGTERM
  <main> at -e:1
1

So that makes the Process.kill approach work just like MRI, which is good (so we have a decent enough workaround for that.) However, the raise SignalException, 'SIGTERM' still gives the seemingly incorrect exit code.

Is this by design or is it a bug?

@headius

This comment has been minimized.

Member

headius commented Apr 12, 2018

This is most likely a bug. I believe we are only looking for MainExitException to provide an error code, when we should also be checking for signal exceptions that get reported in return code.

@headius headius closed this in bf4b9c7 Apr 12, 2018

@headius

This comment has been minimized.

Member

headius commented Apr 12, 2018

Ok, I pushed a simple fix for this to master. It should be producing the right return codes for SignalException now.

@perlun I don't think there's a spec for this. Can you add one?

@headius headius added this to the JRuby 9.2.0.0 milestone Apr 12, 2018

@headius

This comment has been minimized.

Member

headius commented Apr 12, 2018

Fixed for 9.2.

perlun pushed a commit to perlun/spec that referenced this issue Apr 12, 2018

Per Lundberg
Process::Status#termsig: Add SignalException spec
We found a case in JRuby where this was not working as expected: jruby/jruby#5134

This spec describes the expected behavior in this scenario. It works on MRI, but fails on the currently released JRuby version (because of the bug, which is fixed in jruby master.)
@perlun

This comment has been minimized.

Contributor

perlun commented Apr 12, 2018

@headius Thank you for that! In return, I added the spec that you suggested: ruby/spec#596

(It works on MRI but fails on jruby 9.1.16.0. Haven't tested with jruby master, but running rvm install jruby-head now so will check once it's done.)

perlun pushed a commit to perlun/spec that referenced this issue Apr 12, 2018

Per Lundberg
Process::Status#termsig: Add SignalException spec
We found a case in JRuby where this was not working as expected: jruby/jruby#5134

This spec describes the expected behavior in this scenario. It works on MRI, but fails on the currently released JRuby version (because of the bug, which is fixed in jruby master.)
@perlun

This comment has been minimized.

Contributor

perlun commented Apr 12, 2018

I tested now with jruby-head. Unfortunately, there still seems to be some issues:

$ ../mspec/bin/mspec /Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb
$ ruby /Volumes/git/3rd-party/mspec/bin/mspec-run /Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb
jruby 9.2.0.0-SNAPSHOT (2.5.0) 2018-04-12 34bec6e Java HotSpot(TM) 64-Bit Server VM 25.152-b16 on 1.8.0_152-b16 +jit [darwin-x86_64]

1)
Process::Status#exitstatus for a child that raised SignalException returns nil FAILED
Expected 143
 to equal nil

/Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb:21:in `block in (root)'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyEnumerable.java:1737:in `all?'
org/jruby/RubyArray.java:1778:in `each'
org/jruby/RubyArray.java:1778:in `each'
/Volumes/git/3rd-party/spec/core/process/status/exitstatus_spec.rb:3:in `<main>'
org/jruby/RubyKernel.java:990:in `load'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyArray.java:1778:in `each'
[/ | ==================100%================== | 00:00:00]      1F      0E

Finished in 5.506023 seconds

1 file, 2 examples, 2 expectations, 1 failure, 0 errors, 0 tagged
$ ../mspec/bin/mspec /Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb
$ ruby /Volumes/git/3rd-party/mspec/bin/mspec-run /Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb
jruby 9.2.0.0-SNAPSHOT (2.5.0) 2018-04-12 34bec6e Java HotSpot(TM) 64-Bit Server VM 25.152-b16 on 1.8.0_152-b16 +jit [darwin-x86_64]

1)
Process::Status#termsig for a child that raised SignalException returns the signal FAILED
Expected nil
 to equal 15

/Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb:23:in `block in (root)'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyEnumerable.java:1737:in `all?'
org/jruby/RubyArray.java:1778:in `each'
org/jruby/RubyArray.java:1778:in `each'
/Volumes/git/3rd-party/spec/core/process/status/termsig_spec.rb:3:in `<main>'
org/jruby/RubyKernel.java:990:in `load'
org/jruby/RubyBasicObject.java:2596:in `instance_eval'
org/jruby/RubyArray.java:1778:in `each'
[- | ==================100%================== | 00:00:00]      1F      0E

Finished in 5.705038 seconds

1 file, 3 examples, 3 expectations, 1 failure, 0 errors, 0 tagged

These issues are closely related, but they probably are in the Process::Status class. Do you want me to file another issue about it or can we reopen this one?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment