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

[Kevin Rushforth]

This seems like something that could be useful, although at this point 
in the release we would more likely do it for JDK 10. I do note that 
including the class that made the illegal access is generally a good 
idea when that class is attempting the access on its own behalf. For 
frameworks such as JavaFX beans or FXML, we are making the request on 
behalf of an application, it wasn't as helpful to have the class itself 
be in the error message (which was Mandy's main point). So there are at 
least two cases to consider if we end up creating such a utility method.

Thanks.

-- Kevin


Gregg Wonderly wrote:
> If there is not already such an exception, it would seem like a good idea to have an exception that formats such a message from constructor parameters providing the details so that it’s the same everywhere, and so that it can be changed in once place if needed.
>
> Gregg
>
>   
>> On May 3, 2017, at 9:48 AM, Kevin Rushforth <kevin.rushforth at oracle.com> 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
>>>
>>>  
>>>       
>
>