#LayerPrimitives should be re-opened

[David M. Lloyd]

On 02/24/2017 12:58 PM, Thomas Watson wrote:
> Comments inline.
>
> Tom
>
>> From: "David M. Lloyd" <david.lloyd at redhat.com>
>> I am requesting that the #LayerPrimitives issue be re-opened.
>>
>> This issue requested the ability for Layer.Controller to perform the
>> following operations, which Modules can already do for themselves (and,
>> by extension, Layers can do as well using bytecode injection):
>>
>> • addExports(Module source, String packageName, Module target) -
>> equivalent to Module#addExports(String packageName, Module target),
>> "source" must be in the controller's corresponding layer
>> • addUses(Module source, Class<?> service) - equivalent to
>> Module#addUses(Class<?> service), "source" must be in the controller's
>> corresponding layer
>>
>> These two methods should be added because requiring bytecode injection
>> to perform these functions is unreasonably onerous, which alone should
>> be enough to justify re-opening the issue.
>
> This seems reasonable to me given that a module can perform these
> operations themselves it makes sense to allow the controller to do it also.
>
> I think the addUses would be more useful if we had a way to know when a
> module is trying to load a new ServiceLoader class.  This way we could
> dynamically add the required uses just in time before the loader did its
> check to see if the module can use the service.

I think that service loading in general might be an unresolved issue. 
Earlier I recall we discussed letting the Layer have some jurisdiction 
over what services were available and how to find them, which relates 
closely to this, but I don't see anything non-internal that could enable 
either this feature or being able to detect when a service is being 
loaded.  And, discovering undeclared service consumers is non-trivial.

>> The following self-explanatory operations do not have corresponding
>> methods on Module, and normally can only be done by editing the
>> descriptor or using the Instrumentation#redefineModule(...) API to do so:
>>
>> • addOpensToAll(Module source, String packageName)
>> • addExportsToAll(Module source, String packageName)
>>
>> It is not always possible for a container to know what packages need to
>> be exported or opened to all when the module is built, especially in a
>> dynamic deployment environment like ours.  Note that
>> Controller#addOpens(Module source, String packageName, Module target) is
>> already an existent method on Controller thus is already deemed "safe".
>>
>> The following operations neither have corresponding methods on Module,
>> nor can be done by editing the descriptor AFAIK:
>>
>> • addOpensToAllUnnamed(Module source, String packageName)
>> • addExportsToAllUnnamed(Module source, String packageName)
>>
>> The latter is arguably not necessary since unnamed modules are usually
>> not going to be bound by JPMS linkage, so from what I understand, the
>> exports usually won't apply in this case anyway.
>
> This is not my understanding.  Unnamed modules can read any other
> module, but they still can only load classes from packages that are
> unqualified exports already.  But I do question if the addExportsToAll
> and addOpensToAll include unnamed modules or not.  If they do then it
> seems this extra method to only do it for unnamed modules is not needed.

AFAIK they do include unnamed modules.  The hypothetical case for 
unnamed modules would relate to dealing with mixed systems with 
pre-existing class loaders.  It might be possible in some cases to 
single out the unnamed module for class loaders in such cases and use 
addExports directly in this case.

>> It seems to me that
>> the former method might be of use in some cases; having unnamed module
>> code reflecting on module code does not seem very outlandish to me,
>> especially in a container environment.
>
> Perhaps, but I wonder why the controlling layer did not just make the
> modules within the layer open modules so it did not have to do this
> dynamically later.

This is the case where the layer does not know that the module needed to 
be open until some of the module was already loaded.

>> This leaves addPackage(Module source, String packageName).  This method
>> is not available on Module and can only otherwise be done by editing the
>> descriptor or using Instrumentation#redefineModule(...).  However, if
>> your loader does not yet know the complete set of packages (i.e. it's
>> lazy), or the set of packages might be supplemented after the module is
>> initially created, you either need this method, or you have to redesign
>> your container's linkage strategy from the ground up.  We definitely
>> need this method.  One OSGi implementer has indicated that they can live
>> without it (by not implementing some optional pieces, and mandating
>> strict behavior where other containers had flexibility previously) but
>> I've also reached out to a few others to see if I can get more data
>> about this.  I know that JBoss OSGi does presently rely on this sort of
>> functionality already; I am not sure if it is limited to fragment
>> support or if there is more to it.  I have a query out to the maintainer
>> of that project (Arcadiy Ivanov) to get further information, but I
>> understand he is traveling at the moment.
>
> This method would help for the dynamic fragment attachment case in OSGi.
>  Also required for that would be the ability to addExportToAll.  Having
> addPackage without addExportToAll would be an incomplete solution for
> the dynamic fragment attachment case in OSGi..

-- 
- DML