Issue2656

On Java 9:

PS bugs> jython -J--illegal-access=warn
WARNING: Illegal reflective access by org.python.core.PySystemState (file:/C:/Jython/2.7.2a1/jython.jar) to method java.io.Console.encoding()
WARNING: Illegal reflective access by jnr.posix.JavaLibCHelper (file:/C:/Jython/2.7.2a1/jython.jar) to method sun.nio.ch.SelChImpl.getFD()
WARNING: Illegal reflective access by jnr.posix.JavaLibCHelper (file:/C:/Jython/2.7.2a1/jython.jar) to field sun.nio.ch.FileChannelImpl.fd
WARNING: Illegal reflective access by jnr.posix.JavaLibCHelper (file:/C:/Jython/2.7.2a1/jython.jar) to field java.io.FileDescriptor.fd
WARNING: Illegal reflective access by jnr.posix.JavaLibCHelper (file:/C:/Jython/2.7.2a1/jython.jar) to field java.io.FileDescriptor.handle
Jython 2.7.2a1+ (default:d74f8c2cd56f, Feb 24 2018, 17:18:53)
[Java HotSpot(TM) 64-Bit Server VM (Oracle Corporation)] on java9.0.1

By 2.7.2 it would be nice not to be the poster-child for this new JDK feature:
https://docs.oracle.com/javase/9/migrate/#GUID-7BB28E4D-99B3-4078-BDC4-FC24180CE82B

The infamous one is fixed in jnr.posix (https://github.com/jnr/jnr-posix/issues/108), but others follow if one gives -J--illegal-access=warn. I feel we should at least fix the ones we're directly responsible for.

A reasonably complete set of cases (made by running the regrtest and a bit of clean-up):

By jnr.posix.JavaLibCHelper (jython.jar) to field java.io.FileDescriptor.fd
By jnr.posix.JavaLibCHelper (jython.jar) to field java.io.FileDescriptor.handle
By jnr.posix.JavaLibCHelper (jython.jar) to field sun.nio.ch.FileChannelImpl.fd
By jnr.posix.JavaLibCHelper (jython.jar) to method sun.nio.ch.SelChImpl.getFD()
By jnr.posix.util.FieldAccess (jython.jar) to field java.io.FileDescriptor.fd
By org.python.core.PyBeanProperty (jython.jar) to method com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.setEntityResolver(org.xml.sax.EntityResolver)
By org.python.core.PyBeanProperty (jython.jar) to method com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.setErrorHandler(org.xml.sax.ErrorHandler)
By org.python.core.PyReflectedFunction (jython.jar) to field java.lang.ProcessImpl.handle
By org.python.core.PyReflectedFunction (jython.jar) to method
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl.newSAXParser()
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.jaxp.SAXParserFactoryImpl.setFeature(java.lang.String,boolean)
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.getFeature(java.lang.String)
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.parse(org.xml.sax.InputSource)
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl$JAXPSAXParser.setFeature(java.lang.String,boolean)
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.jaxp.SAXParserImpl.getXMLReader()
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.setContentHandler(org.xml.sax.ContentHandler)
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.org.apache.xerces.internal.parsers.AbstractSAXParser.setDTDHandler(org.xml.sax.DTDHandler)
By org.python.core.PyReflectedFunction (jython.jar) to method com.sun.tools.javac.api.JavacTaskImpl.call()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.net.www.protocol.jar.JarURLConnection.getDefaultUseCaches()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.net.www.protocol.jar.JarURLConnection.setDefaultUseCaches(boolean)
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.Big5.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.Big5.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.Big5_HKSCS.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.Big5_HKSCS.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.EUC_CN.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.EUC_CN.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.EUC_JP.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.EUC_JP.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.EUC_KR.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.EUC_KR.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.GB18030.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.GB18030.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.IBM942.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.IBM942.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.IBM949.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.IBM949.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.IBM950.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.IBM950.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.ISO2022_JP.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.ISO2022_JP.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.ISO2022_JP_2.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.ISO2022_JP_2.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.ISO2022_KR.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.ISO2022_KR.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.SJIS_0213.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.ext.SJIS_0213.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.GBK.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.GBK.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.Johab.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.Johab.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.MS936.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.SJIS.newDecoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.nio.cs.SJIS.newEncoder()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.security.x509.X509CertImpl.toString()
By org.python.core.PyReflectedFunction (jython.jar) to method sun.util.calendar.ZoneInfo.getRawOffset()
By org.python.core.PySystemState (jython.jar) to method java.io.Console.encoding()
By org.python.modules.gc (jython.jar) to field java.util.ArrayList.elementData
By org.python.netty.util.internal.ReflectionUtil (jython.jar) to constructor java.nio.DirectByteBuffer(long,int)
By org.python.netty.util.internal.ReflectionUtil (jython.jar) to field sun.nio.ch.SelectorImpl.publicSelectedKeys
By org.python.netty.util.internal.ReflectionUtil (jython.jar) to field sun.nio.ch.SelectorImpl.selectedKeys
By org.python.netty.util.internal.ReflectionUtil (jython.jar) to method java.nio.Bits.unaligned()

Upgrading jnr-posix to 3.0.44 has reduced the number of occurrences. test_java_integration passes, as Mark Reinhold's shouting no longer drowns out Rover's 42 barks.

In other places, I'm beginning to wonder if we can really quiet the code. If we insist on getting a real C API file descriptor, and it only exists in a private field, it's bound to involve private members like java.io.FileDescriptor.fd and sun.nio.ch.SelChImpl.getFD(). See also #2679, peeled off from this issue. Maybe jnr.posix, netty and the like will have to provide a DLL in the way JLine does?

Ideally, Jython would run perfectly well on a standard Java API anything further is an opt-in.  We've always stayed very close to that ideal.

Updating netty helps too. https://hg.python.org/jython/rev/ead1cae7f8de

The list currently produced from:
    dist\bin\jython -J--illegal-access=warn -m test.regrtest -e >refacc.log 2>&1
and in bash:
    cat refacc.log | grep reflective | sort | uniq
with a bit of clean up is:

WARNING: An illegal reflective access operation has occurred
By jnr.posix.JavaLibCHelper$ReflectiveAccess (jnr-posix-3.0.44.jar) to field java.io.FileDescriptor.fd
By jnr.posix.JavaLibCHelper$ReflectiveAccess (jnr-posix-3.0.44.jar) to field java.io.FileDescriptor.handle
By jnr.posix.JavaLibCHelper$ReflectiveAccess (jnr-posix-3.0.44.jar) to field sun.nio.ch.FileChannelImpl.fd
By jnr.posix.JavaLibCHelper$ReflectiveAccess (jnr-posix-3.0.44.jar) to method sun.nio.ch.SelChImpl.getFD()
By jnr.posix.util.FieldAccess (jnr-posix-3.0.44.jar) to field java.io.FileDescriptor.fd
By org.python.core.PyJavaType (jython-dev.jar) to method java.lang.Object.finalize()
By org.python.core.PyReflectedFunction (jython-dev.jar) to field java.lang.ProcessImpl.handle
By org.python.modules.gc (jython-dev.jar) to field java.util.ArrayList.elementData

Yay! In Java 9, Process has a pid() method so we no longer have to steal java.lang.ProcessImpl.handle

https://hg.python.org/jython/rev/61eda8fd9356

The remaining violations that surface from jnr-posix are from org.python.core.io.FileIO.__int__, or from jnr.posix.JavaLibCHelper$ReflectiveAccess. In both cases as a result of calls to os.fstat (that is org.python.modules.posix.PosixModule$FstatFunction.__call__).

For the most part, these in turn are the result of calls to _pyio.open where it attempts to get the filesystem block size, although there are other root causes (fileinput$py.readline, SimpleHTTPServer$py.send_head). Note that it is only _pyio.open that does this. the pure Java io.open has a fixed idea of the ideal block size.

Seems like issues #1736 and #2320 just go on giving. C file descriptors are a terrible idea. They don't *describe* anything unless you also have access to a table we can't see. As we have worked this a few times already I incline to think that a good solution is not imminent, so not for 2.7.2.

The last four really intrusive reflective access errors all come from this line where we check whether an apparent filename is actually a tty:
https://hg.python.org/jython/file/61eda8fd9356/src/org/python/util/jython.java#l355

However, there seems no easy fix here without first clarifying the logic of the run() method as a whole. See #2686.

We are now down to these few, of which only the last two are within our control:

By jnr.posix.JavaLibCHelper$ReflectiveAccess to field java.io.FileDescriptor.fd
By jnr.posix.JavaLibCHelper$ReflectiveAccess to method sun.nio.ch.SelChImpl.getFD()
By jnr.posix.JavaLibCHelper$ReflectiveAccess to field sun.nio.ch.FileChannelImpl.fd
By jnr.posix.JavaLibCHelper$ReflectiveAccess to field java.io.FileDescriptor.handle
By jnr.posix.util.FieldAccess to field java.io.FileDescriptor.fd

By org.python.core.PyJavaType to method java.lang.Object.finalize()
By org.python.modules.gc to field java.util.ArrayList.elementData

I think the issue in the gc module can be fixed (or better saying "muted") for now by setting "DONT_TRAVERSE_BY_REFLECTION" flag by default.

I.e. there is somewhere the line in the gc module:
private static short gcFlags = 0;
change it to
private static short gcFlags = DONT_TRAVERSE_BY_REFLECTION;

That should avoid illegal access in gc module in most cases.
A proper solution is a todo, however I don't really see a good one right now (maybe via JNI).

Regarding jnr we should make sure these are filed as issues in jnr if not yet fixed in a current version.

Regarding PyJavaType, do you know where in the code the illegal access is triggered? Gave it a quick look, but that seems not to be trivial to identify.

Stefan: Thanks for the tip concerning gc. That sounds an easy solution. I've dug into the traversal before and although I would like to understand it better, a ready-made answer is welcome.

I'm looking at the PyJavaType case now. I find that it is raised by the meth.setAccessible we do when Options.respectJavaAccessibility is false. IMO there is a case for saying that a disrespectful user should expect a dusty reply from Java 9 (=won't fix). It pops out of test_java_accessibility for that explicit reason. Not sure yet why it does so from test_codecs_jy. 

With regard to jnr.posix, I think they've got it in https://github.com/jnr/jnr-posix/pull/109 and https://github.com/jnr/jnr-posix/issues/110. The first of these is interesting, since the resolution only defers the illegal access until it can't be avoided. If the client asks for the C-level file descriptor that is private to the stream (or for the Windows handle), there is no legal way to provide it.

I've managed to eliminate the offending calls from the Jython startup: as in jnr.posix itself, users don't suffer until they explicitly ask for the forbidden data.

> there is no legal way to provide it

Via JNI, access to private stuff might still be legal. Traditionally, JNI calls are not checked at all (not even for type safety). I think this is still the case for Java 9, but I didn't really try it yet. Adding a native part is clumsy for pure Jython, but can be a suitable fallback for a number of scenarios.

Pure Jython should be able to live without knowing the (true) Unix numeric file descriptor or Windows file handle. I appreciate that a C-extension may depend on the actual value so there needs to be a means for JNI (or JyNI) to reach it.

Agreed about the file descriptor. I know we discussed that in another issue a while ago. I was more thinking of a way to preserve the option
python.security.respectJavaAccessibility=false.

Stefan wrote:
    ...there is somewhere the line in the gc module:
    private static short gcFlags = 0;
    change it to
    private static short gcFlags = DONT_TRAVERSE_BY_REFLECTION;

Yes, that stops the warnings. But now the tests fail (of course) because the expected collection does not take place. If we disable traversal by reflection as a whole, then we must skip the test.

We could do this either on all JVMs or on all those from Java 9 onwards. But that can't be rational, since if this is necessary on Java 8 it doesn't suddenly stop being necessary on Java 9.

Here I risk displaying my ignorance of the use case. The best course seems to me to disable it (by default) on all JVMs as shown, and raise an issue about the feature producing warnings with Java 9+. Those who enable reflective traversal (by talking to the gc module), and then get messages that mention reflection and the gc module, should be able to put 2 and 2 together, unlike those for whom it comes out of the blue.

Right, the test needs consideration. This should only affect one single test: test_TraverseByReflection in test_gc_jy.py.
Traversal by reflection was experimental from the start and is only a fallback for PyObjects that neither implement Traverseproc nor are marked as @Untraversable (all Jython-included PyObjects comply). It is unlikely that someone uses advanced gc features while not taking care of his/her custom PyObjects to comply with traverseproc rules. Traverse by reflection was just meant as a fallback and debug feature to simplify adoption of traverseproc rules and advanced GC features in Jython 2.7.
So I guess it is rarly used at best and anyway only fallbag/debug feature. I think we can safely disable it by default and skip this single test.

If we want to preserve the test for Java 7/8 we would have to remove DONT_TRAVERSE_BY_REFLECTION in corresponding
setUpClass (via gc.removeJythonGCFlags). tearDownClass automatically restores the flags previously saved on setUpClass.

However I'd be fine with either solution - all JVMs or pre/post Java9 only.

I still believe this feature could be restored via JNI but given the rare use cases (debugging issues related to gc behavior differing from CPython) I wouldn't attempt to unless a compelling reason shows up.

In line with this good advice, I've opted to preserve the test in 7/8:
https://hg.python.org/jython/rev/dc1ffb710882
While the mode of traversal is available to users, we should test it (which we do since we run on Java 7 onwards). But maybe --add-opens is what we should have here, once we do not support 8?

On the topic of --add-opens, and going back to "... thinking of a way to preserve the option python.security.respectJavaAccessibility=false", this change provides an example of a test in which I chose to let the illegal reflective access go ahead, but tell java to expect it:
https://hg.python.org/jython/rev/677009e86ce2

I have also fixed the warnings that arise from codec in:
https://hg.python.org/jython/rev/5820f8a915f1
Here, the "respectJavaAccessibility" had been made unnecessary by earlier work (or was never necessary, maybe). The test had a number of unrelated problems that I've fixed too.

The only reflective access warnings now are from jnr.posix, and what I called "fundamental". That is, we explicitly ask for information Java just doesn't give up any other way.

Removing closed dependencies. I consider the substance of this is fixed.

I think dependency #2686 is fixed too apart from that last note I've left myself to check it with non-ascii paths.

On the block size issue, have you looked at java.nio.file.FileStore::getBlockSize?

@alanb It arises between code from the Python stdlib we don't want to change and jnr-posix. It disappears with --illegal-access=deny (since it is wrapped in try...except) so I'm not worried.

It looks almost feasible for us to have an os module in pure Java these days. As so often discussed, integer file descriptors are the snag, requiring some kind of (JyNI-compatible) cheat if Java is determined not to expose them. Not in 2.7.2, however.