Re: ModuleLayer.Controller strongly references Module

[Michał Kłeczek]

> On 29 Nov 2021, at 10:29, Alan Bateman  wrote:
>> 
>> It looks like ML.Controller.addExport(source, pn, target) does not update 
>> the target module ClassLoader view of exported packages in the default 
>> ClassLoader implementation (jdk.internal.loader.Loader).
>> What is the reasoning behind such implementation? I am wondering now how 
>> useful Controller.addExport() really is...
>> 
> The addXXX methods are about accessibility. In the case of addExports then 
> maybe you want to export an internal package so that code in a module in the 
> child layer can make use of (via reflection) public classes in that package.
> 
> You are right that the addXXX methods doesn't do any magic for visibility. 
> For super advanced cases where someone is using their own class loaders then 
> they need to ensure that the class loader delegation supports the readability 
> graph. Same thing when they use addExports where they may need to their class 
> loaders to adjust their delegation to support changes at run-time.

Thanks for explanation.
Unfortunately, I couldn’t find any good in-depth documentation covering Module, 
ModuleLayer and ClassLoader interaction. Could you point me to anything useful?

In my case I wanted to avoid implementing my own custom ClassLoader but 
actually doing that allows for easier management of ML.Controller lifetime: the 
ClassLoader can keep the reference to it.

In the end I ended up with one-to-one relationship between ML, Module and 
custom ClassLoader that form a cycle that can be GCed.

Thanks for help.

-- Michal

Re: ModuleLayer.Controller strongly references Module

[Alan Bateman]

On 28/11/2021 22:29, Michał Kłeczek wrote:

:
It looks like ML.Controller.addExport(source, pn, target) does not update the 
target module ClassLoader view of exported packages in the default ClassLoader 
implementation (jdk.internal.loader.Loader).
What is the reasoning behind such implementation? I am wondering now how useful 
Controller.addExport() really is...

The addXXX methods are about accessibility. In the case of addExports 
then maybe you want to export an internal package so that code in a 
module in the child layer can make use of (via reflection) public 
classes in that package.


You are right that the addXXX methods doesn't do any magic for 
visibility. For super advanced cases where someone is using their own 
class loaders then they need to ensure that the class loader delegation 
supports the readability graph. Same thing when they use addExports 
where they may need to their class loaders to adjust their delegation to 
support changes at run-time.


-Alan

Re: ModuleLayer.Controller strongly references Module

[Michał Kłeczek]

> On 24 Nov 2021, at 21:01, Alan Bateman  wrote:
> 
> On 24/11/2021 18:14, Michał Kłeczek wrote:
>> 
>> 
>> For sure it’s fun :)
>> 
>> Regardless of “smart proxies” the lifetime of ML.Controller should be - if 
>> not fixed - at least be documented IMHO. Right now it’s a minefield.
> 
> It's also somewhat unusual so I think we'll need to think if this is the 
> right thing to do.

It looks like ML.Controller.addExport(source, pn, target) does not update the 
target module ClassLoader view of exported packages in the default ClassLoader 
implementation (jdk.internal.loader.Loader).
What is the reasoning behind such implementation? I am wondering now how useful 
Controller.addExport() really is...

-- Michal

Re: ModuleLayer.Controller strongly references Module

[Alan Bateman]

On 24/11/2021 18:14, Michał Kłeczek wrote:



For sure it’s fun :)

Regardless of “smart proxies” the lifetime of ML.Controller should be 
- if not fixed - at least be documented IMHO. Right now it’s a minefield.


It's also somewhat unusual so I think we'll need to think if this is the 
right thing to do.


The ML.Controller for the boot layer is not needed and I think it would 
be attractive nuisance to have it hanging around.


-Alan

Re: ModuleLayer.Controller strongly references Module

[Michał Kłeczek]

> On 24 Nov 2021, at 13:33, Alan Bateman  wrote:
>> 
>> 1. Controller is created upon ML construction and returned from static 
>> ML.defineModules methods.
>> 2. Created two controller liveness tests (they rely on System.gc() though - 
>> not sure if there are some tricks to test it reliably).
> 
> I read your mail about "smart proxy" classes and lazy downloading of modules. 
> I don't have an opinion at this time on whether this is worth doing or not.

For sure it’s fun :)

Regardless of “smart proxies” the lifetime of ML.Controller should be - if not 
fixed - at least be documented IMHO. Right now it’s a minefield.

> 
> If you want to experiment with patches then add a volatile field to ML and 
> only set it when VM.isBooted() is true. In order words, let the ML.Controller 
> for the boot layer be GC'ed as it is now (it would be attractive nuisance to 
> keep it alive). There are many tests in the test/jdk tree that check that a 
> weak ref is cleared, there is also some test infrastructure to help, so you 
> might want to look at those.

See attached patch.

I am now curious why leaving controller for boot layer is to be avoided.

-- Michal

Re: ModuleLayer.Controller strongly references Module

[Alan Bateman]

On 24/11/2021 12:19, Michał Kłeczek wrote:

Hi Alan,

(Disregard previous message - fat fingers)

Attached is a first draft of the patch:

1. Controller is created upon ML construction and returned from static 
ML.defineModules methods.
2. Created two controller liveness tests (they rely on System.gc() 
though - not sure if there are some tricks to test it reliably).


I read your mail about "smart proxy" classes and lazy downloading of 
modules. I don't have an opinion at this time on whether this is worth 
doing or not.


If you want to experiment with patches then add a volatile field to ML 
and only set it when VM.isBooted() is true. In order words, let the 
ML.Controller for the boot layer be GC'ed as it is now (it would be 
attractive nuisance to keep it alive). There are many tests in the 
test/jdk tree that check that a weak ref is cleared, there is also some 
test infrastructure to help, so you might want to look at those.


-Alan

Re: ModuleLayer.Controller strongly references Module

[Michał Kłeczek]

Hi Alan,

(Disregard previous message - fat fingers)

Attached is a first draft of the patch: 

1. Controller is created upon ML construction and returned from static 
ML.defineModules methods.
2. Created two controller liveness tests (they rely on System.gc() though - not 
sure if there are some tricks to test it reliably).

> On 22 Nov 2021, at 21:37, Alan Bateman  > wrote:
>> 
> Assuming it's not exposed, and that there is no reference to the 
> ML.Controller for the boot layer,

I am not sure I understand this. Do you mean ML.controller should be null for 
the boot layer?
This would require null check in static ML.defineModules methods to return new 
Controller for the boot layer (done only once during system init).


-- Michal

Re: ModuleLayer.Controller strongly references Module

[Michał Kłeczek]

Hi Alan,

Attached is a first draft of the patch.
Controller is created upon ML construction and returned from static 
ML.defineModules methods.

> On 22 Nov 2021, at 21:37, Alan Bateman  wrote:
>> 
> Assuming it's not exposed, and that there is no reference to the 
> ML.Controller for the boot layer,

I am not sure I understand this. Do you mean ML.controller should be null for 
the boot layer?
This would require null check in static ML.defineModules methods to return new 
Controller for the boot layer (done only once during system init).


-- Michal

Re: ModuleLayer.Controller strongly references Module

[Michał Kłeczek]

Hi Alan,

> On 22 Nov 2021, at 21:37, Alan Bateman  wrote:
> 
> On 22/11/2021 07:23, Michał Kłeczek wrote:
>> 
>> Would it be too much to ask for accepting a patch adding a reference to 
>> ML.Controller to ModuleLayer?
>> It wouldn’t require changes to the spec I think as the lifetime of 
>> ML.Controller is unspecified.
>> 
> Assuming it's not exposed, and that there is no reference to the 
> ML.Controller for the boot layer, then it might be okay but I think it would 
> be useful to understand the use-case first. As I assume you know, the 
> ML.Controller allows containers to do the equivalent of --add-reads, 
> --add-exports, and --add-opens in child layers.  Since you want to retain the 
> reference to the ML.Controller around then it may be some other reason, maybe 
> opening packages to descendants beyond the child layer?

Yes.

As a hobby project I am revisiting my old idea briefly described here (it’s 
been sooo long ago :) ):
https://markmail.org/message/kte6z7kcgaqcayor 


In short: it is Jini/RMI with codebase annotations being “smart” installers of 
code necessary to deserialise an object.

At that time it wasn’t really feasible to transfer arbitrary objects graphs due 
to hierarchical class loading model.

The general idea is that upon serialisation of an object its class is annotated 
with another object representing the closure of this class’ module and its 
dependencies (subgraph of the application module graph).
To annotate the annotation object I use null (actually the recursion level 
allowed when reading annotations is configurable).

When deserialising I recreate the module graph on the receiving side. Some 
nodes in the dependency graph are replaced with local modules so that the 
receiving side can cast deserialised object to a known type.
Equal annotations (as defined by Object.equals) - even received from different 
sources - always map to the same module on the receiving side.

In non-modular pre-jigsaw world two jar files (server.jar and server-dl.jar) 
were provided: server-dl.jar contained a subset of server.jar classes 
implementing Smart proxy.
In post-jigsaw world Smart proxy and Server reside in different modules. The 
(canonical) structure of a server application is:

Server module --- depends on ---> Smart proxy module > depdends on ---> one 
or more API modules

To make things more encapsulated Proxy module exports its smart proxy class 
package only to the Server module (Smart proxy class has to be public in 
contrast to pre-jigsaw world because split packages are disallowed).

What I would like to support is also (rare) scenario that a superset of already 
instantiated module graph is received and one of the previously downloaded 
modules exports a package to a named module downloaded at a later time. 
(Imagine scenario when a server can be migrated and is transferred after its 
smart proxy).

That’s why I need to keep ML.Controller for later.

Hope that makes it clear :)

-- Michal

Re: ModuleLayer.Controller strongly references Module

[Alan Bateman]

On 22/11/2021 07:23, Michał Kłeczek wrote:


Would it be too much to ask for accepting a patch adding a reference 
to ML.Controller to ModuleLayer?
It wouldn’t require changes to the spec I think as the lifetime of 
ML.Controller is unspecified.


Assuming it's not exposed, and that there is no reference to the 
ML.Controller for the boot layer, then it might be okay but I think it 
would be useful to understand the use-case first. As I assume you know, 
the ML.Controller allows containers to do the equivalent of --add-reads, 
--add-exports, and --add-opens in child layers.  Since you want to 
retain the reference to the ML.Controller around then it may be some 
other reason, maybe opening packages to descendants beyond the child layer?


-Alan

Re: ModuleLayer.Controller strongly references Module

[Michał Kłeczek]

From OpenJDK discuss:

> On 21 Nov 2021, at 21:55, Alan Bateman  wrote:
> 
> On 21/11/2021 13:27, Michał Kłeczek wrote:
>> Hello All,
>> 
>> I am looking for a way to associate ModuleLayer.Controller with a Module (or 
>> ModuleLayer) in such a way that it won’t disable garbage collection of 
>> Modules and ModuleLayers.
>> WeakHashMap won’t work as Controller 
>> strongly references its ModuleLayer (which in turn strongly references 
>> Modules).
>> WeakHashMap> won’t work 
>> either cause controller will be garbage collected.
>> 
>> So far I only came up with subclassing a ModuleReference and keeping the 
>> reference to the controller there. This won’t work in case when 
>> ModuleLayer.configuration() is used a a parent configuration.
>> 
>> Am I missing something obvious here?
> You are correct that a module layer doesn't keep the ML.Controller alive. A 
> ML.Controller is somewhat niche object to expand the readability graph or 
> maybe add additional edges to open modules, and then be thrown away. It 
> sounds like you want to keep it around in order to do further opening when 
> additional child layers come into being. One way you could do this is create 
> a module with a holder class to keep the reference. So rather than keeping 
> the reference in the model world (ModuleReference), instead put it as a field 
> in a reified module. With the right encapsulation it should give you the 
> module layer -> Controller without compromising the integrity of the layers. 
> If you want to discuss further then it's probably best to follow-up on 
> jigsaw-dev rather here.
> 


Indeed - a synthetic module providing a holder service might do the job.

Seems somewhat cumbersome though.

Would it be too much to ask for accepting a patch adding a reference to 
ML.Controller to ModuleLayer?
It wouldn’t require changes to the spec I think as the lifetime of 
ML.Controller is unspecified.

— Michal