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

JRuby 1.7 branch uses java.lang.management.ManagementFactory #4006

Closed
donv opened this Issue Jul 9, 2016 · 14 comments

Comments

Projects
None yet
6 participants
@donv
Member

donv commented Jul 9, 2016

Environment

JRuby 1.7 branch + Ruboto

org.jruby.RubyGC.count now uses java.lang.management.ManagementFactory.getGarbageCollectorMXBeans which is not available on Android. JRuby 1.7.25 does not do this.

Could not find method java.lang.management.ManagementFactory.getGarbageCollectorMXBeans, referenced from method org.jruby.RubyGC.count
unable to resolve static method 6126: Ljava/lang/management/ManagementFactory;.getGarbageCollectorMXBeans ()Ljava/util/List;
D/dalvikvm( 5135):

@donv donv added this to the JRuby 1.7.26 milestone Jul 9, 2016

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Jul 10, 2016

Member

there doesn't seem to be any recent change on jruby-1_7

Member

kares commented Jul 10, 2016

there doesn't seem to be any recent change on jruby-1_7

@donv

This comment has been minimized.

Show comment
Hide comment
@donv

donv Jul 11, 2016

Member

Hi @kares ! Thanks for looking into this.

I have seen the same error with JRuby 1.7.25, so the change in JRuby may be between 1.7.24 and 1.7.25, but may be influenced by using Java 7/8 and different versions of the Android tooling.

The idea that the missing ManagementFactory is the cause came from the log, but the actual failure is this:

W/System.err( 1423): Caused by: java.lang.RuntimeException: Could not load platform constants for OpenFlags
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.getConstants(ConstantResolver.java:216)
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.lookupAndCacheConstant(ConstantResolver.java:117)
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.getConstant(ConstantResolver.java:105)
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.longValue(ConstantResolver.java:168)
W/System.err( 1423):    at jnr.constants.platform.OpenFlags.intValue(OpenFlags.java:30)
W/System.err( 1423):    at org.jruby.RubyFile.createFileClass(RubyFile.java:153)
W/System.err( 1423):    at org.jruby.Ruby.initCore(Ruby.java:1538)
W/System.err( 1423):    at org.jruby.Ruby.bootstrap(Ruby.java:1303)
W/System.err( 1423):    at org.jruby.Ruby.init(Ruby.java:1214)
W/System.err( 1423):    at org.jruby.Ruby.newInstance(Ruby.java:334)

This is taken from this log: https://s3.amazonaws.com/archive.travis-ci.org/jobs/143766143/log.txt

I know reading these logs can be hard, so don't hesitate to ask if you need help navigating.

I can also start a Ruboto branch that focuses testing for this case.

Member

donv commented Jul 11, 2016

Hi @kares ! Thanks for looking into this.

I have seen the same error with JRuby 1.7.25, so the change in JRuby may be between 1.7.24 and 1.7.25, but may be influenced by using Java 7/8 and different versions of the Android tooling.

The idea that the missing ManagementFactory is the cause came from the log, but the actual failure is this:

W/System.err( 1423): Caused by: java.lang.RuntimeException: Could not load platform constants for OpenFlags
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.getConstants(ConstantResolver.java:216)
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.lookupAndCacheConstant(ConstantResolver.java:117)
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.getConstant(ConstantResolver.java:105)
W/System.err( 1423):    at jnr.constants.platform.ConstantResolver.longValue(ConstantResolver.java:168)
W/System.err( 1423):    at jnr.constants.platform.OpenFlags.intValue(OpenFlags.java:30)
W/System.err( 1423):    at org.jruby.RubyFile.createFileClass(RubyFile.java:153)
W/System.err( 1423):    at org.jruby.Ruby.initCore(Ruby.java:1538)
W/System.err( 1423):    at org.jruby.Ruby.bootstrap(Ruby.java:1303)
W/System.err( 1423):    at org.jruby.Ruby.init(Ruby.java:1214)
W/System.err( 1423):    at org.jruby.Ruby.newInstance(Ruby.java:334)

This is taken from this log: https://s3.amazonaws.com/archive.travis-ci.org/jobs/143766143/log.txt

I know reading these logs can be hard, so don't hesitate to ask if you need help navigating.

I can also start a Ruboto branch that focuses testing for this case.

@kares

This comment has been minimized.

Show comment
Hide comment
@kares

kares Jul 11, 2016

Member

Hey! NP - mostly wanted to point out there wasn't really a change (since 1.7.5) in RubyGC (on jruby-1_7)
... there's a catch (Throwable) around the java.lang.management usage in count but we could do a Class.forName("java.lang.Management.xxx") static check up front as well if that helps Ruboto.

Member

kares commented Jul 11, 2016

Hey! NP - mostly wanted to point out there wasn't really a change (since 1.7.5) in RubyGC (on jruby-1_7)
... there's a catch (Throwable) around the java.lang.management usage in count but we could do a Class.forName("java.lang.Management.xxx") static check up front as well if that helps Ruboto.

@trejkaz

This comment has been minimized.

Show comment
Hide comment
@trejkaz

trejkaz Aug 18, 2016

I have this exact same exception on JRuby 9.1.2.0 and on current master.

Caused by: java.lang.RuntimeException: Could not load platform constants for OpenFlags
    at jnr.constants.platform.ConstantResolver.getConstants(ConstantResolver.java:216)
    at jnr.constants.platform.ConstantResolver.lookupAndCacheConstant(ConstantResolver.java:117)
    at jnr.constants.platform.ConstantResolver.getConstant(ConstantResolver.java:105)
    at jnr.constants.platform.ConstantResolver.longValue(ConstantResolver.java:168)
    at jnr.constants.platform.OpenFlags.intValue(OpenFlags.java:30)
    at org.jruby.RubyFile.createFileClass(RubyFile.java:136)
    at org.jruby.Ruby.initCore(Ruby.java:1584)
    at org.jruby.Ruby.bootstrap(Ruby.java:1366)
    at org.jruby.Ruby.init(Ruby.java:1249)
    at org.jruby.Ruby.newInstance(Ruby.java:340)
    at org.jruby.embed.internal.LocalContext.getRuntime(LocalContext.java:117)
    at org.jruby.embed.internal.ThreadSafeLocalContextProvider.getRuntime(ThreadSafeLocalContextProvider.java:82)
    at org.jruby.embed.jsr223.Utils.setWriter(Utils.java:149)
    at org.jruby.embed.jsr223.Utils.preEval(Utils.java:113)
    at org.jruby.embed.jsr223.JRubyEngine.eval(JRubyEngine.java:88)

I get the feeling the issue is in JNR and not JRuby itself, but I couldn't figure out why it wasn't working. It seems like it is trying to resolve a class as a resource (getResource) through the AppClassLoader but it can't be found - even though when I look in the jars the class file is there.

(More investigation: The new JNR issue was introduced in this commit:

jnr/jnr-constants@89cb411

The Class.forName would have returned the class despite the security policy, whereas the new code returns null whether the class exists or not, if a security policy has not granted permission to read from the jar. Which raises a question - do all getResource() calls have to be done in a doPrivileged block? we have a lot of those in our own code too... :/ )

trejkaz commented Aug 18, 2016

I have this exact same exception on JRuby 9.1.2.0 and on current master.

Caused by: java.lang.RuntimeException: Could not load platform constants for OpenFlags
    at jnr.constants.platform.ConstantResolver.getConstants(ConstantResolver.java:216)
    at jnr.constants.platform.ConstantResolver.lookupAndCacheConstant(ConstantResolver.java:117)
    at jnr.constants.platform.ConstantResolver.getConstant(ConstantResolver.java:105)
    at jnr.constants.platform.ConstantResolver.longValue(ConstantResolver.java:168)
    at jnr.constants.platform.OpenFlags.intValue(OpenFlags.java:30)
    at org.jruby.RubyFile.createFileClass(RubyFile.java:136)
    at org.jruby.Ruby.initCore(Ruby.java:1584)
    at org.jruby.Ruby.bootstrap(Ruby.java:1366)
    at org.jruby.Ruby.init(Ruby.java:1249)
    at org.jruby.Ruby.newInstance(Ruby.java:340)
    at org.jruby.embed.internal.LocalContext.getRuntime(LocalContext.java:117)
    at org.jruby.embed.internal.ThreadSafeLocalContextProvider.getRuntime(ThreadSafeLocalContextProvider.java:82)
    at org.jruby.embed.jsr223.Utils.setWriter(Utils.java:149)
    at org.jruby.embed.jsr223.Utils.preEval(Utils.java:113)
    at org.jruby.embed.jsr223.JRubyEngine.eval(JRubyEngine.java:88)

I get the feeling the issue is in JNR and not JRuby itself, but I couldn't figure out why it wasn't working. It seems like it is trying to resolve a class as a resource (getResource) through the AppClassLoader but it can't be found - even though when I look in the jars the class file is there.

(More investigation: The new JNR issue was introduced in this commit:

jnr/jnr-constants@89cb411

The Class.forName would have returned the class despite the security policy, whereas the new code returns null whether the class exists or not, if a security policy has not granted permission to read from the jar. Which raises a question - do all getResource() calls have to be done in a doPrivileged block? we have a lot of those in our own code too... :/ )

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Aug 18, 2016

Member

@trejkaz could catch. so we need to use Class.forName when the SecurityManager does not allow to read from jars. fancy a PR for jnr-constants ?

Member

mkristian commented Aug 18, 2016

@trejkaz could catch. so we need to use Class.forName when the SecurityManager does not allow to read from jars. fancy a PR for jnr-constants ?

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Aug 19, 2016

Member

A jnr-constants PR would be welcome :-) We need to spin a release of it to untangle SOMAXCONN problems (jnr/jnr-constants#16).

Member

headius commented Aug 19, 2016

A jnr-constants PR would be welcome :-) We need to spin a release of it to untangle SOMAXCONN problems (jnr/jnr-constants#16).

@enebo enebo modified the milestones: JRuby 1.7.26, JRuby 1.7.27 Aug 26, 2016

@kares kares added the JRuby 9000 label Jan 17, 2017

oneiros added a commit to oneiros/jruby that referenced this issue Feb 16, 2017

Downgrade jnr-constants.
As part of #4004 in order to work around #4006.
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 15, 2017

Member

I will fix this.

Member

headius commented Mar 15, 2017

I will fix this.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 15, 2017

Member

I came up with the following patch, but I have been unable to reproduce the problem locally with a simple main() added to one of the jnr-constants classes.

diff --git a/src/main/java/jnr/constants/ConstantSet.java b/src/main/java/jnr/constants/ConstantSet.java
index b3afd86..b7db92d 100644
--- a/src/main/java/jnr/constants/ConstantSet.java
+++ b/src/main/java/jnr/constants/ConstantSet.java
@@ -14,8 +14,11 @@
 
 package jnr.constants;
 
+import java.io.InputStream;
 import java.lang.reflect.Field;
 import java.net.URL;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -35,6 +38,8 @@ public class ConstantSet extends AbstractSet<Constant> {
             = new ConcurrentHashMap<String, ConstantSet>();
     private static final Object lock = new Object();
     private static final ClassLoader LOADER;
+    private static final boolean CAN_LOAD_RESOURCES;
+    private static volatile Throwable RESOURCE_READ_ERROR;
 
     static {
         ClassLoader _loader = ConstantSet.class.getClassLoader();
@@ -43,6 +48,38 @@ public class ConstantSet extends AbstractSet<Constant> {
         } else {
             LOADER = ClassLoader.getSystemClassLoader();
         }
+
+        boolean canLoadResources = false;
+        try {
+            URL thisClass = AccessController.doPrivileged(new PrivilegedAction<URL>() {
+                public URL run() {
+                    return LOADER.getResource("jnr/constants/ConstantSet.class");
+                }
+            });
+
+            InputStream stream = thisClass.openStream();
+
+            try {
+                stream.read();
+            } catch (Throwable t) {
+                // save for future reporting, can't read the stream for whatever reason
+                RESOURCE_READ_ERROR = t;
+            } finally {
+                try {
+                    stream.close();
+                } catch (Exception e) {
+                    // ignore
+                }
+            }
+
+            canLoadResources = true;
+        } catch (Throwable t) {
+            if (RESOURCE_READ_ERROR == null) {
+                RESOURCE_READ_ERROR = t;
+            }
+        }
+
+        CAN_LOAD_RESOURCES = canLoadResources;
     }
 
     /**
@@ -88,12 +125,18 @@ public class ConstantSet extends AbstractSet<Constant> {
 
         for (String prefix : prefixes) {
             String fullName = prefix + "." + name;
+            boolean doClass = true;
+
+            if (CAN_LOAD_RESOURCES) {
+                // Reduce exceptions on boot by trying to find the class as a resource first
+                String path = fullName.replace('.', '/') + ".class";
+                URL resource = LOADER.getResource(path);
 
-            // Reduce exceptions on boot by trying to find the class as a resource first
-            String path = fullName.replace('.', '/') + ".class";
-            URL resource = LOADER.getResource(path);
+                // Able to load resources, but couldn't find this, bail out
+                if (resource == null) doClass = false;
+            }
 
-            if (resource != null) {
+            if (doClass) {
                 try {
                     return (Class<Enum>) Class.forName(fullName).asSubclass(Enum.class);
                 } catch (ClassNotFoundException ex) {
Member

headius commented Mar 15, 2017

I came up with the following patch, but I have been unable to reproduce the problem locally with a simple main() added to one of the jnr-constants classes.

diff --git a/src/main/java/jnr/constants/ConstantSet.java b/src/main/java/jnr/constants/ConstantSet.java
index b3afd86..b7db92d 100644
--- a/src/main/java/jnr/constants/ConstantSet.java
+++ b/src/main/java/jnr/constants/ConstantSet.java
@@ -14,8 +14,11 @@
 
 package jnr.constants;
 
+import java.io.InputStream;
 import java.lang.reflect.Field;
 import java.net.URL;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
 import java.util.*;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.ConcurrentMap;
@@ -35,6 +38,8 @@ public class ConstantSet extends AbstractSet<Constant> {
             = new ConcurrentHashMap<String, ConstantSet>();
     private static final Object lock = new Object();
     private static final ClassLoader LOADER;
+    private static final boolean CAN_LOAD_RESOURCES;
+    private static volatile Throwable RESOURCE_READ_ERROR;
 
     static {
         ClassLoader _loader = ConstantSet.class.getClassLoader();
@@ -43,6 +48,38 @@ public class ConstantSet extends AbstractSet<Constant> {
         } else {
             LOADER = ClassLoader.getSystemClassLoader();
         }
+
+        boolean canLoadResources = false;
+        try {
+            URL thisClass = AccessController.doPrivileged(new PrivilegedAction<URL>() {
+                public URL run() {
+                    return LOADER.getResource("jnr/constants/ConstantSet.class");
+                }
+            });
+
+            InputStream stream = thisClass.openStream();
+
+            try {
+                stream.read();
+            } catch (Throwable t) {
+                // save for future reporting, can't read the stream for whatever reason
+                RESOURCE_READ_ERROR = t;
+            } finally {
+                try {
+                    stream.close();
+                } catch (Exception e) {
+                    // ignore
+                }
+            }
+
+            canLoadResources = true;
+        } catch (Throwable t) {
+            if (RESOURCE_READ_ERROR == null) {
+                RESOURCE_READ_ERROR = t;
+            }
+        }
+
+        CAN_LOAD_RESOURCES = canLoadResources;
     }
 
     /**
@@ -88,12 +125,18 @@ public class ConstantSet extends AbstractSet<Constant> {
 
         for (String prefix : prefixes) {
             String fullName = prefix + "." + name;
+            boolean doClass = true;
+
+            if (CAN_LOAD_RESOURCES) {
+                // Reduce exceptions on boot by trying to find the class as a resource first
+                String path = fullName.replace('.', '/') + ".class";
+                URL resource = LOADER.getResource(path);
 
-            // Reduce exceptions on boot by trying to find the class as a resource first
-            String path = fullName.replace('.', '/') + ".class";
-            URL resource = LOADER.getResource(path);
+                // Able to load resources, but couldn't find this, bail out
+                if (resource == null) doClass = false;
+            }
 
-            if (resource != null) {
+            if (doClass) {
                 try {
                     return (Class<Enum>) Class.forName(fullName).asSubclass(Enum.class);
                 } catch (ClassNotFoundException ex) {
@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 15, 2017

Member

@kares @mkristian @trejkaz @donv Can one of you test if this patch helps, or provide a simple way to reproduce?

Member

headius commented Mar 15, 2017

@kares @mkristian @trejkaz @donv Can one of you test if this patch helps, or provide a simple way to reproduce?

@mkristian

This comment has been minimized.

Show comment
Hide comment
@mkristian

mkristian Mar 21, 2017

Member

@headius patch looks good. the only thing I wondered if the last Class.forName(fullName) could be Class.forName(fullName, true, LOADER) ? at least both Class.forName methods are treated the same by the SecurityManager.

Member

mkristian commented Mar 21, 2017

@headius patch looks good. the only thing I wondered if the last Class.forName(fullName) could be Class.forName(fullName, true, LOADER) ? at least both Class.forName methods are treated the same by the SecurityManager.

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 23, 2017

Member

@mkristian Good idea, I will make that change.

Member

headius commented Mar 23, 2017

@mkristian Good idea, I will make that change.

headius added a commit to jnr/jnr-constants that referenced this issue Mar 23, 2017

Back off from pre-testing resources if security disallows.
This fixes issues running under a security manager that does not
allow loading resources but which does allow reflectively loading
classes.

See jruby/jruby#4006.

headius added a commit to headius/jruby that referenced this issue Mar 23, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 23, 2017

Member

Pushed #4544 to test this fix.

Member

headius commented Mar 23, 2017

Pushed #4544 to test this fix.

@headius headius closed this in 7b380d6 Mar 24, 2017

headius added a commit that referenced this issue Mar 24, 2017

Merge pull request #4544 from headius/fix-4006
Use jnr-constants 0.9.9-SNAPSHOT to fix #4006.

@headius headius reopened this Mar 24, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 24, 2017

Member

Still needs testing on 1.7.

Member

headius commented Mar 24, 2017

Still needs testing on 1.7.

headius added a commit to headius/jruby that referenced this issue Mar 24, 2017

headius added a commit to headius/jruby that referenced this issue Mar 24, 2017

@headius headius closed this Mar 24, 2017

@headius

This comment has been minimized.

Show comment
Hide comment
@headius

headius Mar 24, 2017

Member

Fixed for both 1.7.27 and 9.1.9.0.

Member

headius commented Mar 24, 2017

Fixed for both 1.7.27 and 9.1.9.0.

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