[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-distro-pkg-dev
Subject:    [icedtea-web] RFC: PR618 - Can't install OpenDJ, JavaWebStart fails with Input stream is null error.
From:       omajid () redhat ! com (Omair Majid)
Date:       2011-01-26 21:44:40
Message-ID: 4D4095C8.4020603 () redhat ! com
[Download RAW message or body]

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.

Cheers,
Omair
-------------- next part --------------
A non-text attachment was scrubbed...
Name: lazy-load-resources-03.patch
Type: text/x-patch
Size: 2512 bytes
Desc: not available
Url : http://mail.openjdk.java.net/pipermail/distro-pkg-dev/attachments/20110126/fcda073d/attachment.bin 

[prev in list] [next in list] [prev in thread] [next in thread]