[RFC] netx: only create one instance of JNLPClassLoader for entire application

[Omair Majid]

On 07/08/2010 02:36 PM, Deepak Bhole wrote:
> * Omair Majid<omajid at redhat.com>  [2010-07-08 14:18]:
>> On 07/08/2010 01:56 PM, Deepak Bhole wrote:
>>> * Omair Majid<omajid at redhat.com>   [2010-07-08 09:59]:
>>>> The attached patch modifies the JNLPClassLoader so only one instance
>>>> of the loader is created per application. This patch also makes the
>>>> classloader keep track of each JNLP file and the resources it loads.
>>>> For applets, it only keeps track of the main applet JNLP (the
>>>> PluginBridge) and uses that.
>>>>
>>>> Any comments?
>>>>
>>>> Cheers,
>>>> Omair
>>>
>>> We already use a single classloader (by merging extension loader paths
>>> into the base loader)...
>>>
>>> What bug required this change?
>>>
>>
>> We merge the paths, but lose all the other information. Consider
>> this: anyplace where the current code does a file.foo(), we dont
>> know which JNLPFile file actually is.
>>
>
> Is there a test case where this is an issue?
>
>> Also, this updated version (which knows about the main application
>> JNLP and the extension JNLPs) will make it easier to implement the
>> new cache for netx that I have been planning. During the
>> application's startup, when the Jars are being downloaded and
>> verified, we can ensure that all of the application's jars are
>> treated the same way and we present a consistent cache to the
>> application. In the current implementation sincle multiple
>> classloaders are created, each JNLPClassLoader considres the jars
>> individually for updates and caching.
>>
>
> We should discuss the new cache system. I imagined all of the
> downloading and verification would be moved into a separate caching
> subsystem, in which case jnlpclassloaders wouldn't be doing any
> downloading, so multiple loaders wouldn't matter.
>
> Lets discuss that tomorrow.
>
>> Still, because of how this patch affects the ClassLoader, I would
>> appreciate it people could test it out for a bit before it commit
>> it.
>>
>
> That is my concern as well. JNLPClassLoader is a very critical file, and
> doing this sort of invasive change invalidates a lot of the 'public
> testing' it has gone though over the years.
>

So after discussing the proposed patch offline with Deepak, here is the 
updated patch that includes slight changes.

Cheers,
Omair
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: icedtea6-jnlp-single-classloader-reviewed.patch
Url: http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20100720/e08077a4/icedtea6-jnlp-single-classloader-reviewed.patch