8181087: Module system implementation refresh (6/2017 update)

Alan Bateman Alan.Bateman at oracle.com
Thu Jun 15 07:34:58 UTC 2017



On 15/06/2017 06:37, Mandy Chung wrote:
> :
> java/lang/Class.java
> 2453                     reflectionFactory.getExecutableSharedParameterTypes(method),
>
> reflectionFactory should be accessed via getReflectionFactory().  I see Peter
> already comments on this.
Thanks, it should be getReflectionFactory().


>
> java/lang/Module.java
>   901     void implAddOpensToAllUnnamed(Iterator<String> iterator) {
>   902         if (jdk.internal.misc.VM.isModuleSystemInited()) {
>   903             iterator.forEachRemaining(pn ->
>   904                 implAddExportsOrOpens(pn, ALL_UNNAMED_MODULE, true, true));
>   905             return;
>   906         }
>
> AFAICT this should only be called during module system initialization.
> When will this method be called after initPhase 2?
It's for use during startup (initPhase2) only. If called later then it 
works as if the module somehow reflectively opened the packages to all 
unnamed modules. I wouldn't object to changing it to throwing an 
exception (assuming that is what you are thinking) as the JDK doesn't 
have any use for this after initPhase2.

>
> jdk/internal/loader/Loader.java
>   406     public Enumeration<URL> getResources(String name) throws IOException {
>   407         // this loader
>   408         List<URL> urls = findResourcesAsList(name);
>   409
>   410         // parent loader
>   411         parent.getResources(name).asIterator().forEachRemaining(urls::add);
>   412
>   413         return Collections.enumeration(urls);
>   414     }
>
> We could consider returning an Enumeration that defers finding resources
> from parent class loader after iterating the local resources.
Yes, this is possible but has a lot of potential issues. The new stream 
returning resources() methods is better for doing lazy consumption and 
it could override this (although still lots of potential issues, esp. 
when running with a security manager, concerns about context change like 
TCCL, etc.).


> :
>
> ModulePath.java
>   408     private static final Attributes.Name AUTOMATIC_MODULE_NAME
>   409         = new Attributes.Name("Automatic-Module-Name");
>
> Should this be defined in java.util.jar.Attributes.Name?
As I recall, this was deliberately not included in the automatic name 
proposal.


>
> ServiceLoader.java
>
> 1174                     } else if (!isAssignable(clazz, serviceName)) {
> 1175                         fail(service, clazz.getName() + " not a subtype”);
>
> Peter raises the question on isAssignable that I think it should look at the interfaces recursively.  On the other hand, I wonder if this should simply fail if service.isAssignableFrom(clazz) returns false (which I think it’s the existing JDK 8 behavior).
>
Yes, I think I will remove this.

-Alan.


More information about the hotspot-runtime-dev mailing list