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

Support JRuby on Java >= 9 #141

Merged
merged 2 commits into from
Jan 23, 2019
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
7 changes: 7 additions & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ before_install:
- ruby -e "system('gem update --system') if Gem::Version.new(RUBY_VERSION) >= Gem::Version.new('2.3')"
- gem install bundler --version '~> 1.17'

before_script:
- 'export JAVA_OPTS="${JAVA_OPTS_FOR_SPECS}"'

env:
global:
matrix:
Expand All @@ -36,6 +39,10 @@ matrix:
- rvm: jruby-9.1.9.0
- rvm: ruby-head
- env: "CHILDPROCESS_POSIX_SPAWN=true"
include:
- rvm: jruby-9.2.5.0
jdk: openjdk11
env: "JAVA_OPTS_FOR_SPECS='--add-opens java.base/java.io=org.jruby.dist --add-opens java.base/sun.nio.ch=org.jruby.dist'"
Copy link
Collaborator

@sds sds Jan 22, 2019

Choose a reason for hiding this comment

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

I might be missing a nuance of how .travis.yml treats env, but could you remove the before_script and just have the following in your include:

env:
  - "JAVA_OPTS='--add-opens java.base/java.io=org.jruby.dist --add-opens java.base/sun.nio.ch=org.jruby.dist'"

...or does that not work for some reason? I ask because it would avoid setting JAVA_OPTS="" for all other builds.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Several of the setup commands that run through jruby (and therefore create a JVM) do not need those options, so this was an attempt to only provide them to the invocation that runs rspec. I am open to other options.

exclude:
# Travis does not provide 1.9.3 on OSX
- rvm: 1.9.3
Expand Down
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,6 +173,8 @@ ChildProcess.logger = logger
## Caveats

* With JRuby on Unix, modifying `ENV["PATH"]` before using childprocess could lead to 'Command not found' errors, since JRuby is unable to modify the environment used for PATH searches in `java.lang.ProcessBuilder`. This can be avoided by setting `ChildProcess.posix_spawn = true`.
* With JRuby on Java >= 9, the JVM may need to be configured to allow JRuby to access neccessary implementations; this can be done by adding `--add-opens java.base/java.io=org.jruby.dist` and `--add-opens java.base/sun.nio.ch=org.jruby.dist` to the `JAVA_OPTS` environment variable that is used by JRuby when launching the JVM.


# Implementation

Expand Down
57 changes: 41 additions & 16 deletions lib/childprocess/jruby/process.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,24 +46,49 @@ def wait
end
end

#
# Only supported in JRuby on a Unix operating system, thanks to limitations
# in Java's classes
#
# @return [Integer] the pid of the process after it has started
# @raise [NotImplementedError] when trying to access pid on non-Unix platform
#
def pid
if @process.getClass.getName != "java.lang.UNIXProcess"
raise NotImplementedError, "pid is only supported by JRuby child processes on Unix"
# Implementation of ChildProcess::JRuby::Process#pid depends heavily on
# what Java SDK is being used; here, we look it up once at load, then
# define the method once to avoid runtime overhead.
normalised_java_version_major = java.lang.System.get_property("java.version")
.slice(/^(1\.)?([0-9]+)/, 2)
.to_i
if normalised_java_version_major >= 9

# On modern Javas, we can simply delegate through to `Process#pid`,
# which was introduced in Java 9.
#
# @return [Integer] the pid of the process after it has started
# @raise [NotImplementedError] when trying to access pid on platform for
# which it is unsupported in Java
def pid
@process.pid
rescue java.lang.UnsupportedOperationException => e
raise NotImplementedError, "pid is not supported on this platform: #{e.message}"
end

else

# On Legacy Javas, fall back to reflection.
#
# Only supported in JRuby on a Unix operating system, thanks to limitations
# in Java's classes
#
# @return [Integer] the pid of the process after it has started
# @raise [NotImplementedError] when trying to access pid on non-Unix platform
#
def pid
if @process.getClass.getName != "java.lang.UNIXProcess"
raise NotImplementedError, "pid is only supported by JRuby child processes on Unix"
end

# About the best way we can do this is with a nasty reflection-based impl
# Thanks to Martijn Courteaux
# http://stackoverflow.com/questions/2950338/how-can-i-kill-a-linux-process-in-java-with-sigkill-process-destroy-does-sigter/2951193#2951193
field = @process.getClass.getDeclaredField("pid")
field.accessible = true
field.get(@process)
end

# About the best way we can do this is with a nasty reflection-based impl
# Thanks to Martijn Courteaux
# http://stackoverflow.com/questions/2950338/how-can-i-kill-a-linux-process-in-java-with-sigkill-process-destroy-does-sigter/2951193#2951193
field = @process.getClass.getDeclaredField("pid")
field.accessible = true
field.get(@process)
end

private
Expand Down