Feedback on open modules and packages

Thu Apr 27 20:00:09 UTC 2017

2017/4/12 11:46:04 -0700, Scott Stark <sstark at redhat.com>:
> Relating to open modules and packages as defined in the current public review
> documents, we are seeing a lot of runtime errors when users begin a migration
> to using JMPS for their application when the modules they are interacting
> with make use of reflection. Two broad categories of this are frameworks like
> JavaFX which use reflection to determine which callback methods are
> implemented, and various data binding frameworks.
> 
> Creating a simple JavaFX application with the main class in a module requires
> the following to compile:
> module jfxapp {
>     requires javafx.graphics;
>     requires javafx.controls;
> }
> 
> However, this is not sufficient at runtime as the following exception is seen:
>   
> java -p out/production/Java9 -m jfxapp/org.jboss.fx.Appplication
> Exception in Application constructor
> Exception in thread "main" java.lang.reflect.InvocationTargetException
> 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> 	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> 	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> 	at java.base/java.lang.reflect.Method.invoke(Method.java:563)
> 	at java.base/sun.launcher.LauncherHelper$FXHelper.main(LauncherHelper.java:913)
> Caused by: java.lang.RuntimeException: Unable to construct Application instance: class org.jboss.fx.Appplication
> 	at javafx.graphics/com.sun.javafx.application.LauncherImpl.launchApplication1(LauncherImpl.java:972)
> 	at javafx.graphics/com.sun.javafx.application.LauncherImpl.lambda$launchApplication$2(LauncherImpl.java:200)
> 	at java.base/java.lang.Thread.run(Thread.java:844)
> Caused by: java.lang.IllegalAccessException: class com.sun.javafx.application.LauncherImpl (in module javafx.graphics) cannot access class org.jboss.fx.Appplication (in module jfxapp) because module jfxapp does not export org.jboss.fx to module javafx.graphics
> 	at java.base/jdk.internal.reflect.Reflection.newIllegalAccessException(Reflection.java:370)
> 	at java.base/java.lang.reflect.AccessibleObject.checkAccess(AccessibleObject.java:589)
> 	at java.base/java.lang.reflect.Constructor.newInstance(Constructor.java:479)
> 	at javafx.graphics/com.sun.javafx.application.LauncherImpl.lambda$launchApplication1$8(LauncherImpl.java:884)
> 	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runAndWait$11(PlatformImpl.java:449)
> 	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$9(PlatformImpl.java:418)
> 	at java.base/java.security.AccessController.doPrivileged(Native Method)
> 	at javafx.graphics/com.sun.javafx.application.PlatformImpl.lambda$runLater$10(PlatformImpl.java:417)
> 	at javafx.graphics/com.sun.glass.ui.InvokeLaterDispatcher$Future.run(InvokeLaterDispatcher.java:96)
> 
> Process finished with exit code 1
> 
> To work around this the one has to either make the jfxapp module open or at a
> minimum, open the package which subclasses javafx.application.Application:
> 
> module jfxapp {
>     requires javafx.graphics;
>     requires javafx.controls;
>     opens org.jboss.fx;
> }

Yes, that will work.  You can be even more precise here and open your package
only to the javafx.graphics module, as suggested in the exception message:

    module jfxapp {
        requires javafx.graphics;
        requires javafx.controls;
        opens org.jboss.fx to javafx.graphics;
    }

Since Java FX requires the `Application` subclass to be public, and have a
public no-argument constructor, `exports` would work here just as well.

(In some cases, due to bug in Java FX, you must use unqualified `opens` or
 `exports` directives [1].  We expect to fix that in a future release.)

We don't expect application developers to have to figure this all out for
themselves.  Developers of reflective libraries and frameworks will, going
forward, need to document whatever expectations they have of user modules.
We've just done that for FX, in the JDK 9 EA build (167) published earlier
today:

  http://download.java.net/java/jdk9/jfxdocs/javafx/application/Application.html
  http://download.java.net/java/jdk9/jfxdocs/javafx/fxml/FXML.html

> Cannot there be a syntax that the javafx.* module author uses to declare that
> read access will be required by any package that requires it so that this

(I think you mean "module that requires it", since packages do not themselves
 depend upon anything else.)

> situation can be caught at compile time rather than having to fix access
> problems at runtime? What if there was a syntax along the lines of:
> 
> module javafx.graphics {
>     exports javafx.pplication reflectively;
> ...
> }
> 
> that allowed the compiler to verify that the jfxapp module was correctly
> configured to use the javafx.graphics module?

The property "this code requires reflective access to any module that uses it"
would be better expressed as a property of a module rather than a particular
package, since it might be the case that only code in an internal, non-exported
package actually initiates a module's reflective operations.  Perhaps something
along the lines of:

    reflective module javafx.graphics {
        exports javafx.application;
        ...
    }

Given this information the compiler could then suggest that any module that
requires this module should be open.  It's often the case, however, that only
one or two packages of a module need to be open for such reflective access.
Advising developers to open all of a module's packages when that's not actually
necessary would lead to the unnecessary exposure of internal classes, so I
don't think this suggestion is worth pursuing.

> The issue is more difficult for automatic modules on the class path. A simple

(I think you mean "automatic modules on the module path".  To use a JAR file
 as an automatic module you must place it on the module path; a JAR file on
 the class path is not treated as an automatic module.)

> GSON binding application sees the same type of problem, but in this
> situation, the gson-2.8.0.jar has been promoted to a module by the Java
> runtime.
> 
> java -p /Users/starksm/Dev/Java/Java9/gson/target/classes:/Users/starksm/.m2/repository/com/google/code/gson/gson/2.8.0/gson-2.8.0.jar -m org.jboss.gson/org.jboss.gson.Main
> Exception in thread "main" java.lang.reflect.InaccessibleObjectException: Unable to make public org.jboss.gson.model.BagOfPrimitives() accessible: module org.jboss.gson does not "exports org.jboss.gson.model" to module gson
> 	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:337)
> 	at java.base/java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:281)
> 	at java.base/java.lang.reflect.Constructor.checkCanSetAccessible(Constructor.java:192)
> 	at java.base/java.lang.reflect.Constructor.setAccessible(Constructor.java:185)
> 	at gson at 2.8.0/com.google.gson.internal.ConstructorConstructor.newDefaultConstructor(ConstructorConstructor.java:101)
> 	at gson at 2.8.0/com.google.gson.internal.ConstructorConstructor.get(ConstructorConstructor.java:83)
> 	at gson at 2.8.0/com.google.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:99)
> 	at gson at 2.8.0/com.google.gson.Gson.getAdapter(Gson.java:423)
> 	at gson at 2.8.0/com.google.gson.Gson.toJson(Gson.java:661)
> 	at gson at 2.8.0/com.google.gson.Gson.toJson(Gson.java:648)
> 	at gson at 2.8.0/com.google.gson.Gson.toJson(Gson.java:603)
> 	at gson at 2.8.0/com.google.gson.Gson.toJson(Gson.java:583)
> 	at org.jboss.gson/org.jboss.gson.Main.main(Main.java:13)
> 
> There is no module gson module author in this case, so it cannot be caught at
> compile time. Shouldn't this be handled at runtime as an implicit grant of
> readability from the org.jboss.gson.model package to the gson module due to
> the fact that the gson module was loaded automatically from the class path
> rather than the module path?

Readability is a relationship between two modules, not between a package and
a module.  A package can be exported or opened unconditionally, or else with
qualification to a specific set of modules.  The combination of readability
and exports/opens determines accessibility [2].

In your example the `gson` module is loaded from the module path, and is thus
treated as an automatic module.  You don't say whether the `org.jboss.gson`
module is explicit (i.e., has a `module-info.class` file) or automatic, but
it must be explicit since if it were automatic then it would be open and the
exception shown above would not be thrown.

To eliminate the exception in this scenario we could revise the definition of
automatic modules to say that any module that requires an automatic module will
implicitly open all of its packages to that automatic module.  That would,
however, add significant complexity to the meaning of the `requires` directive.
It would go from (informally) "this module needs that module" to something like
"this module needs that module, and if that module is automatic then change
this module to an 'open' module".  This change would also add work in the long
run: When the `gson` module is explicitly modularized you'll have to either
change your `org.jboss.gson` module into an explicitly `open` module, or else
figure out exactly which of its packages should be `open` to the `gson` module.
Since you've chosen to make `org.jboss.gson` an explicit module at the start,
you might as well just figure that out now.

- Mark

[1] http://mail.openjdk.java.net/pipermail/jigsaw-dev/2017-April/012202.html
[2] http://openjdk.java.net/projects/jigsaw/spec/sotms/#accessibility