[icedtea-web] RFC: Patch to fix PR895

[Omair Majid]

On 03/09/2012 05:19 PM, Deepak Bhole wrote:
> Hi,
> 
> Attached patch fixes PR895:
> "IcedTea-Web searches for missing classes on each loadClass or findClass"
> 
> http://icedtea.classpath.org/bugzilla/show_bug.cgi?id=895

The bug doesn't seem to have any additional information. Is there a test
case or a reproducer? Do you know why the server is being hammered? If
the actual issue is under our control (for example, we are attempting to
re-download stuff we already downloaded), it might be better to fix this
underlying issue.

> ChangeLog:
> 2012-03-09  Deepak Bhole <dbhole at redhat.com>
> 
>     * netx/net/sourceforge/jnlp/runtime/JNLPClassLoader.java: Added new
>     notFoundClasses list.
>     (loadClass): Record if a class is not found and on next call for that
>     class, return immediately.
>     (findClass): Same.
> 

Can you please include a test case for icedtea-web? I think it would be
nice if we could tell if/when this issue reappears later. I would be
fine with either unit tests or jnlp tests.

Some other thoughts:

Won't this patch break applications that use the following pattern?
(Assume the application has enough permissions to download the jar)
- Try and load a class
- If that fails, download a new jar
- Add new jar to classpath
- Try to load the class again.

I am not sure how many applications do this; perhaps it's a non-issue.

The pattern of adding a new line before every return looks scary. I am
quite sure the next person to touch this method and add a new return
will miss adding the code to update notFoundClasses. How about something
like this instead:

- rename findClass to findClassNoCacheCheck (or a better name ;) )
- add a new method findClass():

protected Class findClass(String name) throws ClassNotFoundException {
  if (notFoundClasses.contains(name)) {
    throw new ClassNotFoundException;
  }

  try {
    Class klass = findClassNoCacheCheck(name);
    return klass;
  } catch (ClassNotFoundException cnfe) {
    notFoundClasses.add(name);
    throw cnfe;
  }
}

Cheers,
Omair