Issue2656

classification
Title: Illegal reflective access warnings from Java 9
Type: behaviour Severity: normal
Components: Core Versions: Jython 2.7
Milestone: Jython 2.7.2
process
Status: open Resolution: accepted
Dependencies: Determine console encoding without access violation (Java 9), IllegalAccessException accessing public abstract method via PyReflectedFunction, Re-align the logic of util.jython.run with CPython 2.7 equivalent
View: 2659

View: 2662

View: 2686
Superseder:
Assigned To: jeff.allen Nosy List: jeff.allen, stefan.richthofer
Priority: high Keywords: Java Roadmap

Created on 2018-03-11.09:29:59 by jeff.allen, last changed 2018-10-23.07:23:29 by jeff.allen.

Messages
msg11778 (view) Author: Jeff Allen (jeff.allen) Date: 2018-03-11.09:29:58
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.
msg11780 (view) Author: Jeff Allen (jeff.allen) Date: 2018-03-11.13:03:25
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()
msg11951 (view) Author: Jeff Allen (jeff.allen) Date: 2018-05-07.07:36:43
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.
msg11952 (view) Author: Jeff Allen (jeff.allen) Date: 2018-05-07.12:18:05
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
msg11965 (view) Author: Jeff Allen (jeff.allen) Date: 2018-05-08.22:17:45
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
msg11967 (view) Author: Jeff Allen (jeff.allen) Date: 2018-05-10.07:56:28
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.
msg11992 (view) Author: Jeff Allen (jeff.allen) Date: 2018-05-17.19:54:10
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.
msg12145 (view) Author: Jeff Allen (jeff.allen) Date: 2018-10-19.22:16:04
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
msg12146 (view) Author: Stefan Richthofer (stefan.richthofer) Date: 2018-10-19.23:55:49
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.
msg12147 (view) Author: Jeff Allen (jeff.allen) Date: 2018-10-20.07:57:00
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.
msg12150 (view) Author: Stefan Richthofer (stefan.richthofer) Date: 2018-10-21.14:08:54
> 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.
msg12152 (view) Author: Jeff Allen (jeff.allen) Date: 2018-10-21.17:18:53
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.
msg12153 (view) Author: Stefan Richthofer (stefan.richthofer) Date: 2018-10-21.19:34:03
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.
msg12155 (view) Author: Jeff Allen (jeff.allen) Date: 2018-10-22.07:20:16
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.
msg12156 (view) Author: Stefan Richthofer (stefan.richthofer) Date: 2018-10-22.14:11:19
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.
msg12157 (view) Author: Jeff Allen (jeff.allen) Date: 2018-10-23.07:23:27
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.
History
Date User Action Args
2018-10-23 07:23:29jeff.allensetmessages: + msg12157
2018-10-22 14:11:20stefan.richthofersetmessages: + msg12156
2018-10-22 07:20:17jeff.allensetresolution: accepted
messages: + msg12155
severity: minor -> normal
2018-10-21 19:34:03stefan.richthofersetmessages: + msg12153
2018-10-21 17:18:54jeff.allensetmessages: + msg12152
2018-10-21 14:08:54stefan.richthofersetmessages: + msg12150
2018-10-20 07:57:02jeff.allensetmessages: + msg12147
2018-10-19 23:55:50stefan.richthofersetnosy: + stefan.richthofer
messages: + msg12146
2018-10-19 22:16:05jeff.allensetmessages: + msg12145
2018-08-13 19:58:07jeff.allensetdependencies: + Re-align the logic of util.jython.run with CPython 2.7 equivalent
2018-05-17 19:54:10jeff.allensetmessages: + msg11992
2018-05-10 07:56:29jeff.allensetmessages: + msg11967
2018-05-08 22:17:45jeff.allensetmessages: + msg11965
2018-05-07 12:18:05jeff.allensetmessages: + msg11952
2018-05-07 07:36:43jeff.allensetmessages: + msg11951
2018-04-01 08:07:44jeff.allensetdependencies: + Determine console encoding without access violation (Java 9), - Detail message is not set on PyException from PythonInterpreter
2018-04-01 08:07:19jeff.allensetdependencies: + Detail message is not set on PyException from PythonInterpreter, IllegalAccessException accessing public abstract method via PyReflectedFunction
2018-03-23 20:29:35jeff.allensettitle: Illegal reflective access warnings form Java 9 -> Illegal reflective access warnings from Java 9
2018-03-11 13:03:25jeff.allensetmessages: + msg11780
2018-03-11 13:02:54jeff.allensetmessages: - msg11779
2018-03-11 13:00:19jeff.allensetmessages: + msg11779
2018-03-11 09:30:11jeff.allensetkeywords: + Java Roadmap
2018-03-11 09:30:00jeff.allencreate