[9] Review request: 8177566: FX user module gets IllegalAccessException from sun.reflect.misc.Trampoline

[Kevin Rushforth]

Forgot to respond to this:

> Also in MethodUtil::invoke
>   61            if (!module.isExported(packageName)) {
> You can do this check only if module.isNamed. 

No, this check will work fine on an unnamed module. Module::isExported 
and Module::isOpen always return true for Module::unnamed.

-- Kevin



Kevin Rushforth wrote:
> OK, I'll create a more informative message. I think it will be more 
> clear in the message to just say that it needs to "open" the package 
> to javafx.base, since that would be the recommendation for a package 
> that isn't already exported unconditionally. I'll send out a new 
> webrev this morning with all feedback incorporated.
>
> -- Kevin
>
>
> Mandy Chung wrote:
>>> On May 2, 2017, at 2:22 PM, Kevin Rushforth 
>>> <kevin.rushforth at oracle.com> wrote:
>>>
>>> Here is the message:
>>>
>>> IllegalAccessException: class com.sun.javafx.property.MethodHelper 
>>> cannot access class com.foo (in module foo.app) because module 
>>> foo.app does not open com.foo to javafx.base
>>>     
>>
>> It would be better to emit a more informative message e.g. 
>> “javafx.base cannot access class com.foo (in module foo.app).  Either 
>> exports com.foo unqualifiedly or open com.foo to javafx.base”.  Also 
>> in MethodUtil::invoke
>>   61            if (!module.isExported(packageName)) {
>> You can do this check only if module.isNamed.
>>
>>  
>>> It is roughly the same message that any similar illegal access would 
>>> generate (e.g., we get similar error messages when FXML tries to 
>>> call setAccessible for a field annotated with @FXML if the module is 
>>> not "open" to javafx.fxml).
>>>
>>>    
>>>> javafx.base/src/main/java/com/sun/javafx/property/MethodHelper.java
>>>> javafx.fxml/src/main/java/com/sun/javafx/fxml/MethodHelper.java
>>>> javafx.web/src/main/java/com/sun/webkit/MethodHelper.java
>>>>   45     public static Object invoke(Method m, Object obj, Object[] 
>>>> params)
>>>>
>>>> To avoid 3 ModuleHelper classes, the invoke method can take
>>>> the callerModule argument to replace this line:   56         final 
>>>> Module thisModule = MethodHelper.class.getModule();
>>>>         
>>> I'm fairly certain that won't work. Module::addOpens is caller 
>>> sensitive and will only work when called from the module in question.
>>>
>>>     
>>
>> You are right.  Wont’t work.
>>  
>>>> javafx.base/src/main/java/com/sun/javafx/reflect/MethodUtil.java
>>>>   There are a few other public methods which I think JavaFX doesn’t
>>>>   need and can be removed.
>>>>         
>>> Yes, I could do this to reduce the public footprint of the class. To 
>>> minimize the diffs between the original and our copy, I might just 
>>> comment out the "public". That would also make it easier to add them 
>>> back in a future version (e.g., to eventually get rid of all 
>>> dependency on sun.reflect.misc). Thoughts?
>>>     
>>
>> I will leave it up to you.
>> Mandy
>>
>>