[icedtea-web] RFC: PR618 - Can't install OpenDJ, JavaWebStart fails with Input stream is null error.

[Omair Majid]

On 01/26/2011 04:44 PM, Omair Majid wrote:
> On 01/25/2011 07:03 PM, Dr Andrew John Hughes wrote:
>> On 14:42 Tue 25 Jan , Omair Majid wrote:
>>> Hi,
>>>
>>> The attached patch fixes PR618. The issue is that jars marked as lazy
>>> are not currently searched for resources other than classes. So loading
>>> classes from lazy jars works, but loading anything else (zip files in
>>> the case of the bug report) fails.
>>>
>>> The attached patch fixes findResources to use lazy jars if needed.
>>>
>>> URLClassLoader invokes findResource() as a part of its getResource()
>>> implementation. Instead of duplicating the addition of lazy jars in
>>> getResource(), getResource() was removed, and findResource added
>>> instead. findResource() now delegates to findResources(), so there is
>>> only one place where an actual search for resources is performed.
>>>
>>> Any concerns or comments?
>>>
>>
>>> diff -r 64da2a80df88
>>> netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
>>> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Jan
>>> 25 10:19:20 2011 -0500
>>> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Tue Jan
>>> 25 14:42:07 2011 -0500
>>> @@ -1086,27 +1087,42 @@
>>> * Finds the resource in this, the parent, or the extension
>>> * class loaders.
>>> */
>>> - public URL getResource(String name) {
>>> - URL result = super.getResource(name);
>>> -
>>> - for (int i = 1; i< loaders.length; i++)
>>> - if (result == null)
>>> - result = loaders[i].getResource(name);
>>> -
>>> - return result;
>>> + @Override
>>> + public URL findResource(String name) {
>>> + try {
>>> + return findResources(name).nextElement();
>>> + } catch (NoSuchElementException e) {
>>> + return null;
>>> + } catch (IOException e) {
>>> + return null;
>>> + }
>>
>> Would it be more efficient to check if there are more elements first
>> rather than throwing and catching an exception?
>>
>
> I am not sure about efficiency, but the attached patch fixes it.
>
>>> @@ -1116,7 +1132,7 @@
>>> resources.add(e.nextElement());
>>> }
>>>
>>> - return resources.elements();
>>> + return resources;
>>> }
>>
>> Is findResources the only consumer of the resources collection? If so,
>> it would be
>> more efficient to use an ArrayList or LinkedList and avoid the
>> implicit synchronisation
>> of Vector.
>>
>
> Yup, findResources is the only method using it. I did not want to touch
> original code (at least code that worked), so I left the vector as it
> was. I have changed it now.
>
>> What is the reason for returning a List from findResourcesBySearching
>> rather than just
>> an Iterator/Enumeration? AFAICS, you just create an enumeration from
>> it anyway and
>> hasNext()/hasNextElement() would tell you if there are no entries.
>>
>
> Is using an Iterator/Enumeration better than using a List? I am not
> aware of the differences, and findResourcesBySearching() already creates
> a List that I can return. So I thought I would avoid creating an
> Enumeration based on it until necessary. The attached patch changes this
> bit too, though I am not sure I see the advantage.
>

Patch updated to apply to head.

Thanks,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lazy-load-resources-04.patch
Type: text/x-patch
Size: 2871 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110314/767fffba/lazy-load-resources-04.patch