Module.addOpens should log if the package has been opened for illegal access for the caller

[Johannes Kuhn]

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