[icedtea-web] RFC: PR618 - Can't install OpenDJ, JavaWebStart fails with Input stream is null error.
Dr Andrew John Hughes
ahughes at redhat.com
Wed Mar 16 13:40:54 PDT 2011
On 11:07 Mon 14 Mar , Omair Majid wrote:
> 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.
> >
Ok. I think it would be more efficient than generating an exception.
> >>> @@ -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.
>
It restricts what the receiver can do with the List. With just an Iterator,
they can only look at and remove elements (and you can disable the latter too
if necessary). With a List, they have a lot more access they probably don't need.
> Thanks,
> Omair
> diff -r 5454292b3fae netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java
> --- a/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Thu Mar 10 15:42:01 2011 -0500
> +++ b/netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java Mon Mar 14 11:07:15 2011 -0400
> @@ -40,7 +40,6 @@
> import java.util.List;
> import java.util.Map;
> import java.util.TreeSet;
> -import java.util.Vector;
> import java.util.jar.JarEntry;
> import java.util.jar.JarFile;
>
> @@ -1123,31 +1122,54 @@
> /**
> * Finds the resource in this, the parent, or the extension
> * class loaders.
> + *
> + * @return a <code>URL</code> for the resource, or <code>null</code>
> + * if the resource could not be found.
> */
> - public URL getResource(String name) {
> - URL result = super.getResource(name);
> + @Override
> + public URL findResource(String name) {
> + URL result = null;
>
> - for (int i = 1; i < loaders.length; i++)
> - if (result == null)
> - result = loaders[i].getResource(name);
> + try {
> + Enumeration <URL> e = findResources(name);
> + if (e.hasMoreElements()) {
> + result = e.nextElement();
> + }
> + } catch (IOException e) {
> + // continue
> + }
>
> // If result is still null, look in the codebase loader
> if (result == null && codeBaseLoader != null)
> - result = codeBaseLoader.getResource(name);
> + result = codeBaseLoader.findResource(name);
>
> return result;
> }
>
> /**
> - * Finds the resource in this, the parent, or the extension
> + * Find the resources in this, the parent, or the extension
> * class loaders.
> */
> @Override
> public Enumeration<URL> findResources(String name) throws IOException {
> - Vector<URL> resources = new Vector<URL>();
> + Enumeration<URL> resources = findResourcesBySearching(name);
> +
> + // if not found, load all lazy resources; repeat search
> + while (!resources.hasMoreElements() && addNextResource() != null) {
> + resources = findResourcesBySearching(name);
> + }
> +
> + return resources;
> + }
> +
> + private Enumeration<URL> findResourcesBySearching(String name) throws IOException {
> + List<URL> resources = new ArrayList<URL>();
> Enumeration<URL> e;
>
> for (int i = 0; i < loaders.length; i++) {
> + // TODO check if this will blow up or not
> + // if loaders[1].getResource() is called, wont it call getResource() on
> + // the original caller? infinite recursion?
>
> if (loaders[i] == this)
> e = super.findResources(name);
> @@ -1166,7 +1188,7 @@
> resources.add(e.nextElement());
> }
>
> - return resources.elements();
> + return Collections.enumeration(resources);
At the top, resources has a type List<URL>. So why not just call resources.iterator()?
> }
>
> /**
--
Andrew :)
Free Java Software Engineer
Red Hat, Inc. (http://www.redhat.com)
Support Free Java!
Contribute to GNU Classpath and IcedTea
http://www.gnu.org/software/classpath
http://icedtea.classpath.org
PGP Key: F5862A37 (https://keys.indymedia.org/)
Fingerprint = EA30 D855 D50F 90CD F54D 0698 0713 C3ED F586 2A37
More information about the distro-pkg-dev
mailing list