Jigsaw EA feedback on running an internal app

[Alex Buckley]

Hi Peter,

On 9/11/2015 8:52 AM, Peter Levart wrote:
> Spotted an error in "The State of the Module System" document at:
> http://openjdk.java.net/projects/jigsaw/spec/sotms/ ... In the "Implied
> readability" section, it writes:
>
>> The java.sql.Driver interface, in particular, declares the public method
>>
>> public Logger getParentLogger();
>>
>> where Logger is a type declared in the exported java.util.logging
>> package of the java.logging module.
>>
>> Suppose that code in the com.foo.app module invokes this method in
>> order to acquire a logger and log a message:
>>
>> String url = ...;
>> Properties props = ...;
>> Driver d = DriverManager.getDriver(url);
>> Connection c = d.connect(url, props);
>> d.getParentLogger().info("Connection acquired");
>>
>> If the com.foo.app module is declared as above then this will not
>> work: *The getParentLogger method is defined in the Logger class,
>> which is in the java.logging module, which is not read by the
>> com.foo.app module. That class is therefore inaccessible to code in
>> the com.foo.app module and so the invocation of the getParentLogger
>> method will fail, at both compile time and run time.*
>
>
> The correct text should be, I think: "The getParentLogger method defined
> in Driver class has a signature with a return type of Logger class,
> which is in the java.logging module, which is not read by the
> com.foo.app module. That class is therefore inaccessible to code in the
> com.foo.app module and so the invocation of the .info method will fail,
> at both compile time and run time (in case the modules were successfully
> compiled together using legacy compiler mode and then their artifacts
> split into modules)".

You're right.

> The fact is that getParentLogger method can be invoked even though it's
> return type is in an unreadable module if the return type is not used in
> any way in such invocation. For example:
>
> d.getParentLogger().toString();
>
> (the above assumes that .toString() is declared in Object and not
> overridden in Logger). If it is overriden, it can be invoked
> nevertheless with the following trick:
>
> ((Object) d.getParentLogger()).toString().
>
> The important thing is that Logger as a type must not be referenced in
> bytecodes if the bytecodes are in a module that doesn't read
> java.logging module.
>
> Am I correct? I have experimented with the EA and this is my experience.
> Is the prototype working correctly in this respect?

You're right. The Driver::getParentLogger method IS accessible to the 
caller in the com.foo.app module, but the Logger type IS NOT (and so 
neither is the Logger::info method). Of course, the compiler can 
trivially prove that a cast of any class type (e.g. Logger, as given by 
the signature of Driver::getParentLogger) to Object is legal. Given that 
cast, the compiler will emit an invokevirtual Object::toString, and at 
run time the overriding method will be dynamically dispatched -- could 
be in Logger, could be in a subclass of Logger, but the caller doesn't 
need to know (or have access).

> Now just a thought on implied readability. While it is a convenient
> feature, it can be dangerous if not used correctly. I think a module
> should declare it's own dependencies (requires) for all types it
> references explicitly. In above example, it invokes method Loger.info()
> and therefore uses types from module java.logging explicitly.
>
> A case where implied readability would be abused could be described in
> the following example:
>
> - java.sql module uses java.logging internally and also exposes a method
> Driver.getParentLogger() which returns a Logger which is declared in
> java.logging module. Therefore java.sql requires *public* java.logging
> to imply the readability of java.logging for modules that require only
> java.sql and not java.logging explicitly.
> - com.foo.internal module requires java.sql and only uses the part of
> it's API that doesn't need types from java.logging
> - com.foo.internal also uses Logger internally, but fails to declare
> this. It nevertheless works since java.logging is implicitly readable to
> it through declaration in java.sql
> - java.sql module decides to change it's internal use of logging API and
> instead of java.logging starts using org.apache.log4j. It now requires
> *public* org.apache.log4j instead of java.logging - an incompatible
> change, yes, but should not affect com.foo.internal that only uses the
> part of it's API that doesn't use types from java.logging
> - com.foo.internal now fails because it still uses java.logging, but
> implicit readability of it is not provided by java.sql any more. If
> com.foo.internal declared explicitly a dependency on java.logging, such
> configuration would still work.
>
> So while convenient, implied readability should be used with care.

I would hope that an IDE would raise awareness of the readability 
situation at step 3, where the com.foo.internal module uses Logger from 
a module to which it (com.foo.internal) only has implied readability.

That said, the author of the com.foo.internal module isn't doing 
anything wrong. He's relying on the contract offered by the java.sql 
module (including readability to java.logging) and is not under any 
obligation to use 100% of the types exported by the java.sql module.

It's the author of the java.sql module in step 4 who is potentially 
breaking his consumers. Per step 1, java.sql.Driver::getParentLogger has 
a return type from java.logging, and if that changes to come from 
org.apache.log4j instead, it's a source- and binary-incompatible change. 
Far more consumers of java.sql will break than just com.foo.internal.

Anyway, I agree with the substance of your comment, about using implied 
readability with care -- I recommend you send it to the EG 
(jpms-spec-comments at openjdk.java.net).

Alex