Fixes to reduce access warnings on Java 9.

* Our own trySetAccessible that only works on fully-open modules. This will evolve to use module visibility more accurately. * Move inaccessible fields in FilenoUtil to inner class, to make their use lazier. * Add a flag on by default that assumes 0,1,2 for stdio when JRuby is the "main" for this JVM. Avoids the need to reflectively dig out the descriptor on Java 9.
jruby · Oct 12, 2017 · 2781e22 · 2781e22
1 parent 8311cd6
commit 2781e22
Showing 6 changed files with 165 additions and 71 deletions.
diff --git a/core/src/main/java/org/jruby/RubyGlobal.java b/core/src/main/java/org/jruby/RubyGlobal.java
@@ -56,6 +56,7 @@
 import org.jruby.util.KCode;
 import org.jruby.util.OSEnvironment;
 import org.jruby.util.RegexpOptions;
+import org.jruby.util.cli.Options;
 import org.jruby.util.cli.OutputStrings;
 import org.jruby.util.io.ChannelHelper;
 import org.jruby.util.io.EncodingUtils;
@@ -271,12 +272,24 @@ public static void createGlobals(ThreadContext context, Ruby runtime) {
     }
 
     public static void initSTDIO(Ruby runtime, GlobalVariables globals) {
-        RubyIO stdin = RubyIO.prepStdio(
-                runtime, runtime.getIn(), prepareStdioChannel(runtime, STDIO.IN, runtime.getIn()), OpenFile.READABLE, runtime.getIO(), "<STDIN>");
-        RubyIO stdout = RubyIO.prepStdio(
-                runtime, runtime.getOut(), prepareStdioChannel(runtime, STDIO.OUT, runtime.getOut()), OpenFile.WRITABLE, runtime.getIO(), "<STDOUT>");
-        RubyIO stderr = RubyIO.prepStdio(
-                runtime, runtime.getErr(), prepareStdioChannel(runtime, STDIO.ERR, runtime.getErr()), OpenFile.WRITABLE | OpenFile.SYNC, runtime.getIO(), "<STDERR>");
+        RubyIO stdin, stdout, stderr;
+
+        // If we're the main for the process and native stdio is enabled, use default descriptors
+        if (runtime.getInstanceConfig().isHardExit() && Options.NATIVE_STDIO.load()) {
+            stdin = RubyIO.prepStdio(
+                    runtime, runtime.getIn(), new NativeDeviceChannel(0), OpenFile.READABLE, runtime.getIO(), "<STDIN>");
+            stdout = RubyIO.prepStdio(
+                    runtime, runtime.getOut(), new NativeDeviceChannel(1), OpenFile.WRITABLE, runtime.getIO(), "<STDOUT>");
+            stderr = RubyIO.prepStdio(
+                    runtime, runtime.getErr(), new NativeDeviceChannel(2), OpenFile.WRITABLE | OpenFile.SYNC, runtime.getIO(), "<STDERR>");
+        } else {
+            stdin = RubyIO.prepStdio(
+                    runtime, runtime.getIn(), prepareStdioChannel(runtime, STDIO.IN, runtime.getIn()), OpenFile.READABLE, runtime.getIO(), "<STDIN>");
+            stdout = RubyIO.prepStdio(
+                    runtime, runtime.getOut(), prepareStdioChannel(runtime, STDIO.OUT, runtime.getOut()), OpenFile.WRITABLE, runtime.getIO(), "<STDOUT>");
+            stderr = RubyIO.prepStdio(
+                    runtime, runtime.getErr(), prepareStdioChannel(runtime, STDIO.ERR, runtime.getErr()), OpenFile.WRITABLE | OpenFile.SYNC, runtime.getIO(), "<STDERR>");
+        }
 
         if (runtime.getObject().getConstantFromNoConstMissing("STDIN") == null) {
             runtime.defineVariable(new InputGlobalVariable(runtime, "$stdin", stdin), GLOBAL);

diff --git a/core/src/main/java/org/jruby/java/invokers/RubyToJavaInvoker.java b/core/src/main/java/org/jruby/java/invokers/RubyToJavaInvoker.java
@@ -45,8 +45,10 @@
 import org.jruby.javasupport.JavaConstructor;
 import org.jruby.javasupport.ParameterTypes;
 import org.jruby.runtime.Arity;
+import org.jruby.runtime.Helpers;
 import org.jruby.runtime.Visibility;
 import org.jruby.runtime.builtin.IRubyObject;
+import org.jruby.util.cli.Options;
 import org.jruby.util.collections.IntHashMap;
 import org.jruby.util.collections.NonBlockingHashMapLong;
 
@@ -340,9 +342,12 @@ static JavaProxy castJavaProxy(final IRubyObject self) {
     }
 
     static <T extends AccessibleObject> T setAccessible(T accessible) {
+        // TODO: Replace flag that's false on 9 with proper module checks
         if (!accessible.isAccessible() &&
-                !Ruby.isSecurityRestricted() ) {
-            try { accessible.setAccessible(true); }
+                !Ruby.isSecurityRestricted() &&
+                Options.JI_SETACCESSIBLE.load() &&
+                accessible instanceof Member) {
+            try { Helpers.trySetAccessible((Member) accessible); }
             catch (SecurityException e) {}
             catch (RuntimeException re) {
                 rethrowIfNotInaccessibleObject(re);
@@ -362,8 +367,11 @@ private static void rethrowIfNotInaccessibleObject(RuntimeException re) {
     }
 
     static <T extends AccessibleObject> T[] setAccessible(T[] accessibles) {
+        // TODO: Replace flag that's false on 9 with proper module checks
         if (!allAreAccessible(accessibles) &&
-                !Ruby.isSecurityRestricted() ) {
+                !Ruby.isSecurityRestricted() &&
+                Options.JI_SETACCESSIBLE.load() &&
+                allAreMember(accessibles)) {
             try { AccessibleObject.setAccessible(accessibles, true); }
             catch (SecurityException e) {}
             catch (RuntimeException re) {
@@ -378,6 +386,11 @@ private static <T extends AccessibleObject> boolean allAreAccessible(T[] accessi
         return true;
     }
 
+    private static <T extends AccessibleObject> boolean allAreMember(T[] accessibles) {
+        for (T accessible : accessibles) if (!(accessible instanceof Member)) return false;
+        return true;
+    }
+
     protected T findCallable(IRubyObject self, String name, IRubyObject[] args, final int arity) {
         switch (arity) {
             case 0:

diff --git a/core/src/main/java/org/jruby/runtime/Helpers.java b/core/src/main/java/org/jruby/runtime/Helpers.java
@@ -1,6 +1,10 @@
 package org.jruby.runtime;
 
+import java.lang.reflect.AccessibleObject;
 import java.lang.reflect.Array;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Member;
+import java.lang.reflect.Method;
 import java.nio.channels.ClosedChannelException;
 import java.nio.charset.Charset;
 import java.util.ArrayList;
@@ -2869,4 +2873,54 @@ public static boolean respondsToMethod(DynamicMethod method, boolean checkVisibi
         return !(checkVisibility &&
                 (method.getVisibility() == PRIVATE || method.getVisibility() == PROTECTED));
     }
+
+    /**
+     * When running on Java 9 this method will use module openness to do setAccessible, if possible.
+     *
+     * @param member the method, field, or constructor to attempt to setAccessible
+     * @return whether the setAccessible was successful
+     */
+    public static boolean trySetAccessible(Member member) {
+        if (!(member instanceof AccessibleObject)) return false;
+
+        AccessibleObject ao = (AccessibleObject) member;
+
+        if (ao.isAccessible()) return true;
+
+        if (getModule != null) {
+            try {
+                Class<?> declaringClass = member.getDeclaringClass();
+                Object module = getModule.invoke(declaringClass);
+                if ((Boolean) isOpen.invoke(module, declaringClass.getPackage().getName())) {
+                    ao.setAccessible(true);
+                    return true;
+                }
+                return false;
+            } catch (Exception e) {
+                return false;
+            }
+        } else {
+            try {
+                ao.setAccessible(true);
+                return true;
+            } catch (Exception e) {
+                return false;
+            }
+        }
+    }
+
+    private static final Method getModule;
+    private static final Method isOpen;
+    static {
+        Method _getModule = null;
+        Method _isOpen = null;
+        try {
+            _getModule = Class.class.getMethod("getModule");
+            Class module = Class.forName("java.lang.Module");
+            _isOpen = module.getMethod("isOpen", String.class);
+        } catch (Exception e) {
+        }
+        getModule = _getModule;
+        isOpen = _isOpen;
+    }
 }
diff --git a/core/src/main/java/org/jruby/util/cli/Options.java b/core/src/main/java/org/jruby/util/cli/Options.java
@@ -136,6 +136,7 @@ public class Options {
     public static final Option<Integer> FFI_COMPILE_THRESHOLD = integer(NATIVE, "ffi.compile.threshold", 100, "Number of FFI invocations before generating a bytecode stub.");
     public static final Option<Boolean> FFI_COMPILE_INVOKEDYNAMIC = bool(NATIVE, "ffi.compile.invokedynamic", false, "Use invokedynamic to bind FFI invocations.");
     public static final Option<Boolean> FFI_COMPILE_REIFY = bool(NATIVE, "ffi.compile.reify", false, "Reify FFI compiled classes.");
+    public static final Option<Boolean> NATIVE_STDIO = bool(NATIVE, "native.stdio", true, "Use native wrappers around the default stdio descriptors.");
 
     public static final Option<Integer> THREADPOOL_MIN = integer(THREADPOOL, "thread.pool.min", 0, "The minimum number of threads to keep alive in the pool.");
     public static final Option<Integer> THREADPOOL_MAX = integer(THREADPOOL, "thread.pool.max", Integer.MAX_VALUE, "The maximum number of threads to allow in the pool.");
@@ -189,6 +190,7 @@ public class Options {
     public static final Option<Boolean> DUMP_INSTANCE_VARS = bool(DEBUG, "dump.variables", false, "Dump class + instance var names on first new of Object subclasses.");
     public static final Option<Boolean> REWRITE_JAVA_TRACE = bool(DEBUG, "rewrite.java.trace", true, "Rewrite stack traces from exceptions raised in Java calls.");
 
+    // TODO: Replace flag that's false on 9 with proper module checks
     public static final Option<Boolean> JI_SETACCESSIBLE = bool(JAVA_INTEGRATION, "ji.setAccessible", calculateSetAccessibleDefault(), "Try to set inaccessible Java methods to be accessible.");
     public static final Option<Boolean> JI_LOGCANSETACCESSIBLE = bool(JAVA_INTEGRATION, "ji.logCanSetAccessible", false, "Log whether setAccessible is working.");
     public static final Option<Boolean> JI_UPPER_CASE_PACKAGE_NAME_ALLOWED = bool(JAVA_INTEGRATION, "ji.upper.case.package.name.allowed", false, "Allow Capitalized Java package names.");

diff --git a/core/src/main/java/org/jruby/util/io/ChannelHelper.java b/core/src/main/java/org/jruby/util/io/ChannelHelper.java
@@ -12,12 +12,14 @@
 
 import jnr.posix.util.FieldAccess;
 import org.jruby.RubyInstanceConfig;
+import org.jruby.runtime.Helpers;
 
 import java.io.ByteArrayInputStream;
 import java.io.FilterInputStream;
 import java.io.FilterOutputStream;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.lang.reflect.Field;
 import java.nio.channels.Channels;
 import java.nio.channels.ReadableByteChannel;
 import java.nio.channels.WritableByteChannel;
@@ -93,9 +95,9 @@ public static InputStream unwrapBufferedStream(InputStream filteredStream) {
     public static OutputStream unwrapFilterOutputStream(OutputStream filteredStream) {
         while (filteredStream instanceof FilterOutputStream) {
             try {
-                OutputStream tmpStream = (OutputStream)
-                        FieldAccess.getProtectedFieldValue(FilterOutputStream.class,
-                                "out", filteredStream);
+                Field out = FilterInputStream.class.getDeclaredField("out");
+                OutputStream tmpStream =
+                        Helpers.trySetAccessible(out) ? (OutputStream) out.get(filteredStream) : null;
 
                 // try to unwrap as a Drip stream
                 if (!(tmpStream instanceof FilterOutputStream)) {
@@ -131,9 +133,9 @@ public static OutputStream unwrapFilterOutputStream(OutputStream filteredStream)
     public static InputStream unwrapFilterInputStream(InputStream filteredStream) {
         while (filteredStream instanceof FilterInputStream) {
             try {
-                InputStream tmpStream = (InputStream)
-                        FieldAccess.getProtectedFieldValue(FilterInputStream.class,
-                                "in", filteredStream);
+                Field in = FilterInputStream.class.getDeclaredField("in");
+                InputStream tmpStream =
+                        Helpers.trySetAccessible(in) ? (InputStream) in.get(filteredStream) : null;
 
                 // could not acquire
                 if (tmpStream == null) break;
@@ -161,7 +163,8 @@ public static InputStream unwrapFilterInputStream(InputStream filteredStream) {
     private static OutputStream unwrapDripStream(OutputStream stream) {
         if (isDripSwitchable(stream)) {
             try {
-                return (OutputStream) FieldAccess.getProtectedFieldValue(stream.getClass(), "out", stream);
+                Field out = stream.getClass().getDeclaredField("out");
+                return Helpers.trySetAccessible(out) ? (OutputStream) out.get(stream) : null;
             } catch (Exception e) {
             }
         }
@@ -171,7 +174,8 @@ private static OutputStream unwrapDripStream(OutputStream stream) {
     private static InputStream unwrapDripStream(InputStream stream) {
         if (isDripSwitchable(stream)) {
             try {
-                return (InputStream) FieldAccess.getProtectedFieldValue(stream.getClass(), "in", stream);
+                Field in = stream.getClass().getDeclaredField("in");
+                return Helpers.trySetAccessible(in) ? (InputStream) in.get(stream) : null;
             } catch (Exception e) {
             }
         }

diff --git a/core/src/main/java/org/jruby/util/io/FilenoUtil.java b/core/src/main/java/org/jruby/util/io/FilenoUtil.java
@@ -6,6 +6,7 @@
 import jnr.posix.POSIX;
 import jnr.unixsocket.UnixServerSocketChannel;
 import jnr.unixsocket.UnixSocketChannel;
+import org.jruby.runtime.Helpers;
 
 import java.io.FileDescriptor;
 import java.lang.reflect.Field;
@@ -24,30 +25,30 @@ public FilenoUtil(POSIX posix) {
     }
 
     public static FileDescriptor getDescriptorFromChannel(Channel channel) {
-        if (SEL_CH_IMPL_GET_FD != null && SEL_CH_IMPL.isInstance(channel)) {
+        if (ReflectiveAccess.SEL_CH_IMPL_GET_FD != null && ReflectiveAccess.SEL_CH_IMPL.isInstance(channel)) {
             // Pipe Source and Sink, Sockets, and other several other selectable channels
             try {
-                return (FileDescriptor)SEL_CH_IMPL_GET_FD.invoke(channel);
+                return (FileDescriptor) ReflectiveAccess.SEL_CH_IMPL_GET_FD.invoke(channel);
             } catch (Exception e) {
                 // return bogus below
             }
-        } else if (FILE_CHANNEL_IMPL_FD != null && FILE_CHANNEL_IMPL.isInstance(channel)) {
+        } else if (ReflectiveAccess.FILE_CHANNEL_IMPL_FD != null && ReflectiveAccess.FILE_CHANNEL_IMPL.isInstance(channel)) {
             // FileChannels
             try {
-                return (FileDescriptor)FILE_CHANNEL_IMPL_FD.get(channel);
+                return (FileDescriptor) ReflectiveAccess.FILE_CHANNEL_IMPL_FD.get(channel);
             } catch (Exception e) {
                 // return bogus below
             }
-        } else if (FILE_DESCRIPTOR_FD != null) {
+        } else if (ReflectiveAccess.FILE_DESCRIPTOR_FD != null) {
             FileDescriptor unixFD = new FileDescriptor();
 
             // UNIX sockets, from jnr-unixsocket
             try {
                 if (channel instanceof UnixSocketChannel) {
-                    FILE_DESCRIPTOR_FD.set(unixFD, ((UnixSocketChannel)channel).getFD());
+                    ReflectiveAccess.FILE_DESCRIPTOR_FD.set(unixFD, ((UnixSocketChannel)channel).getFD());
                     return unixFD;
                 } else if (channel instanceof UnixServerSocketChannel) {
-                    FILE_DESCRIPTOR_FD.set(unixFD, ((UnixServerSocketChannel)channel).getFD());
+                    ReflectiveAccess.FILE_DESCRIPTOR_FD.set(unixFD, ((UnixServerSocketChannel)channel).getFD());
                     return unixFD;
                 }
             } catch (Exception e) {
@@ -110,7 +111,7 @@ public static int filenoFrom(Channel channel) {
     }
 
     private static int getFilenoUsingReflection(Channel channel) {
-        if (FILE_DESCRIPTOR_FD != null) {
+        if (ReflectiveAccess.FILE_DESCRIPTOR_FD != null) {
             return filenoFrom(getDescriptorFromChannel(channel));
         }
         return -1;
@@ -119,7 +120,7 @@ private static int getFilenoUsingReflection(Channel channel) {
     public static int filenoFrom(FileDescriptor fd) {
         if (fd.valid()) {
             try {
-                return (Integer)FILE_DESCRIPTOR_FD.get(fd);
+                return (Integer) ReflectiveAccess.FILE_DESCRIPTOR_FD.get(fd);
             } catch (Exception e) {
                 // failed to get
             }
@@ -128,60 +129,67 @@ public static int filenoFrom(FileDescriptor fd) {
         return -1;
     }
 
-    static {
-        Method getFD;
-        Class selChImpl;
-        try {
-            selChImpl = Class.forName("sun.nio.ch.SelChImpl");
+    public static final int FIRST_FAKE_FD = 100000;
+    protected final AtomicInteger internalFilenoIndex = new AtomicInteger(FIRST_FAKE_FD);
+    private final Map<Integer, ChannelFD> filenoMap = new ConcurrentHashMap<Integer, ChannelFD>();
+    private final POSIX posix;
+
+    private static class ReflectiveAccess {
+        static {
+            Method getFD;
+            Class selChImpl;
             try {
-                getFD = selChImpl.getMethod("getFD");
-                getFD.setAccessible(true);
+                selChImpl = Class.forName("sun.nio.ch.SelChImpl");
+                try {
+                    getFD = selChImpl.getMethod("getFD");
+                    if (!Helpers.trySetAccessible(getFD)) {
+                        getFD = null;
+                    }
+                } catch (Exception e) {
+                    getFD = null;
+                }
             } catch (Exception e) {
+                selChImpl = null;
                 getFD = null;
             }
-        } catch (Exception e) {
-            selChImpl = null;
-            getFD = null;
-        }
-        SEL_CH_IMPL = selChImpl;
-        SEL_CH_IMPL_GET_FD = getFD;
+            SEL_CH_IMPL = selChImpl;
+            SEL_CH_IMPL_GET_FD = getFD;
 
-        Field fd;
-        Class fileChannelImpl;
-        try {
-            fileChannelImpl = Class.forName("sun.nio.ch.FileChannelImpl");
+            Field fd;
+            Class fileChannelImpl;
             try {
-                fd = fileChannelImpl.getDeclaredField("fd");
-                fd.setAccessible(true);
+                fileChannelImpl = Class.forName("sun.nio.ch.FileChannelImpl");
+                try {
+                    fd = fileChannelImpl.getDeclaredField("fd");
+                    if (!Helpers.trySetAccessible(fd)) {
+                        fd = null;
+                    }
+                } catch (Exception e) {
+                    fd = null;
+                }
             } catch (Exception e) {
+                fileChannelImpl = null;
                 fd = null;
             }
-        } catch (Exception e) {
-            fileChannelImpl = null;
-            fd = null;
-        }
-        FILE_CHANNEL_IMPL = fileChannelImpl;
-        FILE_CHANNEL_IMPL_FD = fd;
-
-        Field ffd;
-        try {
-            ffd = FileDescriptor.class.getDeclaredField("fd");
-            ffd.setAccessible(true);
-        } catch (Exception e) {
-            ffd = null;
-        }
-        FILE_DESCRIPTOR_FD = ffd;
-    }
+            FILE_CHANNEL_IMPL = fileChannelImpl;
+            FILE_CHANNEL_IMPL_FD = fd;
 
-    // FIXME shouldn't use static; may interfere with other runtimes in the same JVM
-    public static final int FIRST_FAKE_FD = 100000;
-    protected final AtomicInteger internalFilenoIndex = new AtomicInteger(FIRST_FAKE_FD);
-    private final Map<Integer, ChannelFD> filenoMap = new ConcurrentHashMap<Integer, ChannelFD>();
-    private final POSIX posix;
+            Field ffd;
+            try {
+                ffd = FileDescriptor.class.getDeclaredField("fd");
+                if (!Helpers.trySetAccessible(ffd)) {
+                    ffd = null;
+                }
+            } catch (Exception e) {
+                ffd = null;
+            }
+            FILE_DESCRIPTOR_FD = ffd;
+        }
 
-    private static final Class SEL_CH_IMPL;
-    private static final Method SEL_CH_IMPL_GET_FD;
-    private static final Class FILE_CHANNEL_IMPL;
-    private static final Field FILE_CHANNEL_IMPL_FD;
-    private static final Field FILE_DESCRIPTOR_FD;
+        private static final Class SEL_CH_IMPL;
+        private static final Method SEL_CH_IMPL_GET_FD;
+        private static final Class FILE_CHANNEL_IMPL;
+        private static final Field FILE_CHANNEL_IMPL_FD;
+        private static final Field FILE_DESCRIPTOR_FD;
+    }
 }