-
-
Notifications
You must be signed in to change notification settings - Fork 925
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
Comments
there doesn't seem to be any recent change on jruby-1_7 |
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:
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. |
Hey! NP - mostly wanted to point out there wasn't really a change (since 1.7.5) in |
I have this exact same exception on JRuby 9.1.2.0 and on current master.
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: 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 could catch. so we need to use |
A jnr-constants PR would be welcome :-) We need to spin a release of it to untangle SOMAXCONN problems (jnr/jnr-constants#16). |
As part of jruby#4004 in order to work around jruby#4006.
I will fix this. |
I came up with the following patch, but I have been unable to reproduce the problem locally with a simple 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) { |
@kares @mkristian @trejkaz @donv Can one of you test if this patch helps, or provide a simple way to reproduce? |
@headius patch looks good. the only thing I wondered if the last |
@mkristian Good idea, I will make that change. |
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.
Pushed #4544 to test this fix. |
Use jnr-constants 0.9.9-SNAPSHOT to fix #4006.
Still needs testing on 1.7. |
Fixed for both 1.7.27 and 9.1.9.0. |
Environment
JRuby 1.7 branch + Ruboto
org.jruby.RubyGC.count
now usesjava.lang.management.ManagementFactory.getGarbageCollectorMXBeans
which is not available on Android. JRuby 1.7.25 does not do this.The text was updated successfully, but these errors were encountered: