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

9.2.8 -- java.util.logging.ConsoleHandler.setOutputStream works in java 8 but not 11 (impacts trinidad) #5821

Closed
cshupp1 opened this issue Aug 9, 2019 · 11 comments

Comments

@cshupp1
Copy link

commented Aug 9, 2019

On Windows 10 using jruby-complete-9.2.8.0-20190808.165920-96.jar,

consider the following irb session:

C:\work\uts-web\uts-web-editor>c:\languages\Java\jdk1.8.0_121\bin\java -jar %JRUBY_JAR% -S jirb
irb(main):001:0> JRUBY_VERSION
=> "9.2.8.0-SNAPSHOT"
irb(main):002:0>     JUL = Java::JavaUtilLogging
=> Java::JavaUtilLogging
irb(main):003:0> handler = JUL::ConsoleHandler.new
=> #<Java::JavaUtilLogging::ConsoleHandler:0x64942607>
irb(main):005:0> handler.setOutputStream(Java::JavaLang::System.out)
=> nil

The same in 11:

irb(main):006:0> JRUBY_VERSION
=> "9.2.8.0-SNAPSHOT"
irb(main):007:0>  JUL = Java::JavaUtilLogging
=> Java::JavaUtilLogging
irb(main):008:0>  handler = JUL::ConsoleHandler.new
=> #<Java::JavaUtilLogging::ConsoleHandler:0x315cf170>
irb(main):009:0>  handler.setOutputStream(Java::JavaLang::System.out)
?[1mTraceback?[m (most recent call last):
        6: from uri:classloader:/META-INF/jruby.home/bin/jirb:13:in `<main>'
        5: from org/jruby/RubyKernel.java:1193:in `catch'
        4: from org/jruby/RubyKernel.java:1193:in `catch'
        3: from org/jruby/RubyKernel.java:1425:in `loop'
        2: from org/jruby/RubyKernel.java:1061:in `eval'
        1: from (irb):9:in `evaluate'
?[1mNoMethodError (?[4mundefined method `setOutputStream' for #<Java::JavaUtilLogging::ConsoleHandler:0x315cf170>?[0;1m)?[m
irb(main):010:0> java.lang.System.getProperty('java.version')
=> "11.0.4"

This affects Trinidad.

Thanks,

Cris

@cshupp1 cshupp1 changed the title java.util.logging.ConsoleHandler.setOutputStream works in java 8 but not 11 (impacts trinidad) 9.2.8 -- java.util.logging.ConsoleHandler.setOutputStream works in java 8 but not 11 (impacts trinidad) Aug 9, 2019

@cshupp1

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

Chicanery like this seems to work:

JUL = Java::JavaUtilLogging

class Poo < JUL::ConsoleHandler

  def setOutputStream(stream)
    super(stream)
  end

end

Poo.new.setOutputStream(java.lang.System.out)
@cshupp1

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

Whoops sorry, the above chicanery works in 8 but not 11, so inheritance may need some work too.

irb(main):057:0> Poo.new.setOutputStream(java.lang.System.out)
?[1mTraceback?[m (most recent call last):
        7: from uri:classloader:/META-INF/jruby.home/bin/jirb:13:in `<main>'
        6: from org/jruby/RubyKernel.java:1193:in `catch'
        5: from org/jruby/RubyKernel.java:1193:in `catch'
        4: from org/jruby/RubyKernel.java:1425:in `loop'
        3: from org/jruby/RubyKernel.java:1061:in `eval'
        2: from (irb):57:in `evaluate'
        1: from (irb):52:in `setOutputStream'
?[1mNoMethodError (?[4msuper: no superclass method `setOutputStream' for #<Poo:0x2bcb1414>?[0;1m)?[m
?[1mDid you mean?  setOutputStream?[m
@headius

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

The issue here is that setOutputStream has protected access (on the superclass StreamHandler), and on Java 9+ we are somewhat limited on what we can do to access non-visible stuff. Our choice up to this point has been to not try to force accessibility if the module is not open, since it will produce those ugly warnings people keep complaining about.

So the simple answer is that you need to open up that package and that module to JRuby.

Unfortunately, there's a bug in JRuby that prevents this from working:

$ JAVA_OPTS="--add-opens java.logging/java.util.logging=org.jruby.dist" \
jruby -e 'java.util.logging.ConsoleHandler.new.setOutputStream(File.open("blah.txt", "w").to_outputstream)'
NoMethodError: undefined method `setOutputStream' for #<Java::JavaUtilLogging::ConsoleHandler:0x2fac80a8>
  <main> at -e:1

The bug in question is that when testing for accessibility, we ask the module subsystem if the target method's class's package is open but do not pass a module or class from JRuby (in the standard JRuby distribution, this module is org.jruby.dist). So even though it has been opened up to JRuby, it's not open to "everyone" and the check fails.

I have a fix for this that's fairly simple:

diff --git a/core/src/main/java/org/jruby/javasupport/binding/Initializer.java b/core/src/main/java/org/jruby/javasupport/binding/Initializer.java
index 70146e5171..5e2834b9d3 100644
--- a/core/src/main/java/org/jruby/javasupport/binding/Initializer.java
+++ b/core/src/main/java/org/jruby/javasupport/binding/Initializer.java
@@ -540,7 +542,7 @@ public abstract class Initializer {
             if ( Modifier.isPrivate(mod) ) continue;
 
             // Skip protected methods if we can't set accessible
-            if ( !Modifier.isPublic(mod) && !Modules.trySetAccessible(method)) continue;
+            if ( !Modifier.isPublic(mod) && !Modules.trySetAccessible(method, Ruby.class)) continue;
 
             // ignore bridge methods because we'd rather directly call methods that this method
             // is bridging (and such methods are by definition always available.)

I'm trying to figure out if there's any way to work around this. It's possible to specify at the command line that a module/package should be exported to "ALL-UNNAMED" modules, but there does not appear to be a way to open it up to "everyone".

@cshupp1

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

@headius But I assume the inheritance example, provided above, you would intend to support right?

@cshupp1

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

BTW -- My goal here is to get a large JRuby project running on Java 11. This task seem rougher than I would have thought. I discovered this trying to get trinidad up.

In file gem_home\gems\trinidad-1.5.0.B2\lib\trinidad\lifecycle\logging.rb changing:

    def self.new_console_handler(stream)
       handler = JUL::ConsoleHandler.new # sets output stream to System.err
      handler.writer = nil if handler.respond_to?(:writer=) # avoid writer.close
      if handler.respond_to?(:sealed) && handler.sealed
        handler.sealed = false # avoid manager security checks
        handler.setOutputStream(stream) # closes previous writer if != null
        handler.sealed = true
      else
        handler.setOutputStream(stream)
      end
       handler
    end

To:

    def self.new_console_handler(stream)
      handler = JUL::ConsoleHandler.new # sets output stream to System.err
      arg_clazzes = [java.io.OutputStream].to_java(java.lang.Class)
      stream_method = JUL::StreamHandler.java_class.to_java.getDeclaredMethod(:setOutputStream, arg_clazzes)
      stream_method.setAccessible(true)
      handler.writer = nil if handler.respond_to?(:writer=) # avoid writer.close
      if handler.respond_to?(:sealed) && handler.sealed
        handler.sealed = false # avoid manager security checks
        stream_method.invoke(handler,stream) # closes previous writer if != null
        handler.sealed = true
      else
        stream_method.invoke(handler,stream)
      end
      puts "All dun Cris! #{stream}"
      handler
    end

Fixes that problem, but opens the next one in the chain.

@headius

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

@cshupp1 It is, and it appears that my fix makes that example work too!

[] ~/projects/jruby $ cat blah.rb
JUL = Java::JavaUtilLogging

class Poo < JUL::ConsoleHandler

  def setOutputStream(stream)
    super(stream)
  end

end

Poo.new.setOutputStream(java.lang.System.out)

[] ~/projects/jruby $ JAVA_OPTS="--add-opens java.logging/java.util.logging=org.jruby.dist" jruby blah.rb

[] ~/projects/jruby $ 

I have not found any way to work around the bug I patched above, unfortunately. I will make the change to JRuby and push it to master.

headius added a commit that referenced this issue Aug 9, 2019

Provide a JRuby class when checking accessibility.
If we do not pass any class here, the module isOpen check will
only return true if the module is open to "everyone". This is only
the case if the module itself opens up a given package with no
restrictions (i.e. it does not specify a target module).

By passing a class, the isOpen check will use whatever module
JRuby is running as, such as org.jruby.dist in the standard
distribution or org.jruby.core in the jruby-core jar. This in turn
makes it possible to open the module up at the command line and
make non-public methods accessible.

This appears to be the best fix we can do for #5821 in a module
environment. It also appears to fix the issues dispatching to
protected superclass methods from a concrete subclass, also shown
in #5821.
@headius

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

@cshupp1 Try out a newer snapshot build (once that commit passes in Travis) and let me know that you can open up the package in question. This should work for any other packages that are having trouble.

I am also discussing with @enebo and @kares how we can make this process easier. Right now, if we won't be able to set a method accessible, we quietly refuse to bind it. I'm proposing that instead, we should install a dummy method that errors with information about how to open up the given package.

@cshupp1

This comment has been minimized.

Copy link
Author

commented Aug 9, 2019

@headius -- Will do. I see Nexus still doesn't have it yet and I will definitely be w/o internet on Monday and Tuesday of next weekl. @enebo tells me you hope to release 9.2.8 on Monday. If I see that does happen I will just try the released version when I return.

@headius

This comment has been minimized.

Copy link
Member

commented Aug 9, 2019

@cshupp1 The build went wonky for some reason but I've kicked it and hopefully it will deploy the snapshots now.

Note I also filed #5824 which discusses the long-forgotten "@file" feature of the standard java executable; you can put whatever JVM options you want in "file" and they'll be added to your command line if you do jruby -J@file ... or similar. I am proposing that we should have a standard per-directory (and perhaps also another globally in your home dir) that will automatically get picked up, but until then you can still use explicit flags plus an "@file" to make these add-opens easier to stomach.

@cshupp1

This comment has been minimized.

Copy link
Author

commented Aug 14, 2019

@headius everything worked for me on java 11 with trinidad

I used the following opts:

set JAVA_OPTS=%JAVA_OPTS% --add-opens java.base/java.io=org.jruby.dist
set JAVA_OPTS=%JAVA_OPTS% --add-opens java.base/java.nio.channels=org.jruby.dist
set JAVA_OPTS=%JAVA_OPTS% --add-opens java.base/sun.nio.ch=org.jruby.dist
set JAVA_OPTS=%JAVA_OPTS% --add-opens java.base/sun.nio.ch=org.jruby.core <-- says it can't find this one though even though some output calls for it
set JAVA_OPTS=%JAVA_OPTS% --add-opens java.logging/java.util.logging=org.jruby.dist

The issue that followed (when I was testing with the snapshot build)
was my inability to get the following require to load:

require 'bundler/setup'

But this issue is no more!

Of course since 9.2.8 is released I am able to keep jruby-jars consistent with the jar file I pass to java, so that might have been it.

Thanks!

Cris

@cshupp1 cshupp1 closed this Aug 14, 2019

@headius

This comment has been minimized.

Copy link
Member

commented Aug 21, 2019

@cshupp1 The org.jruby.core module is supposed to be associated with the Maven jruby-core jar, which someone from the Java side of things might be using to embed JRuby. However it seems like we may never have done this; I will investigate that separately. As long as you are using JRuby via the standard distribution, you should only need org.jruby.dist.

headius added a commit to headius/jruby that referenced this issue Aug 21, 2019

Move all JI setAccessible to trySetAccessible.
The logic previously was depending on the ji.setAccessible
property, which by default would simply refuse to set accessible
*at all* when the Java version was >= "1.9" (also showing this
logic was introduced very early in the Java 9 development cycle.)

That default is removed here, along with a the CAN_SET_ACCESSIBLE
static field in RubyInstanceConfig deprecated and replaced with
the original meaning of this property, simply whether to attempt
setAccessible.

The direct calls to setAccessible have been replaced by the
trySetAccessible in backport9 library that checks if the target
member's module is open to JRuby, allowing users to opt-in to that
reflective access if their application needs it. Previously these
cases still skipped setAccessible just due to being on Java 9+.

Most of the changes here were masked by the property, but I have
also fixed several others that were not guarded by the property
or that were otherwise used in booting JRuby or extending Java
integration.

This fixes jruby#5841 by allowing `java_method` Method objects to
go ahead and trySetAccessible when the module is open.

This relates to jruby#5821, which fixed in 9.2.8 a similar issue where
even an open module would not set fields and methods accessible.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.