On 10-Dec-20 8:59, Alan Bateman wrote:
On 09/12/2020 22:45, Johannes Kuhn wrote:
Hope I got the right mailing list, otherwise please direct me there.
Module.addOpens currently doesn't produce an illegal access warning
if the target package has only be opened for illegal access.
The Module methods were not intended to emit warnings. The warnings
are supposed to be emitted by calls to AccessibleObject methods that
will fail when the default switches to deny illegal reflective access
by default.
An argument could be made that MethodHandles.privateLookupIn also emits
a warning - so an other piece of code that treats "open" vs "open for
illegal access" differently.
Module.addOpens is a 3rd place where it matters that the package is open
and that doesn't emit warnings.
The other uses of Module.isOpen are related to resources, which I didn't
look into yet.
So: Why do all reflective operations (except reflective open) emit a
warning if the package has only been opened for illegal access?
I think your argument is that if any of the standard or JDK modules
opens a package for illegal reflective access then code on the class
path can use Module.addOpens to open the package, which has the
effective of suppressing warnings. That behavior is there to support
the --add-opens command line option.
Using --add-opens to suppress the warning is not really the big point -
if you don't get any warning, then running with --illegal-access=deny
should not break the program.
Or, to put in in an other way: With --add-opens you don't rely on
illegal access anymore.
So I think this needs a bit of thinking time to see if it's worth
trying to do anything here. The default has switched for Java 16 and
the intention is that the --illegal-access option will be removed by a
future JEP. I agree it would be sad to see hacks like this in
production, it would be far better to channel the energy being put
into hacking around warnings to fixing fragile code to not rely on JDK
internals.
-Alan
In short, there are 3 reflective areas where Module.isOpen matters:
AccessibleObject, MethodHandles.privateLookupIn and Module.addOpens.
Module.addOpens doesn't emit a warning, the others do.
If a module has been reflectively opened, then no warning should
emitted, right.
But reflectively opening a package should emit such a warning if you can
only do that because it has been opened to you for illegal access.
As an other note, I think we already did enter warning fatigue - "yeah,
there will be a warning, doesn't matter", "just accept the cookies"...
So the change to have a default of --illegal-access=deny is a good
thing. Hopefully the result is not "just run it with
--illegal-access=permit".
And yeah, I'm a bit late to the party.
- Johannes
