Re: URLClassLoader update

[Mark Wielaard <mark at klomp dot org>]

Hi,

On Tue, 2002-12-03 at 22:14, Mark Wielaard wrote:
> On Tue, 2002-12-03 at 10:41, Tom Tromey wrote:
> > >>>>> "Mark" == Mark Wielaard <mark@klomp.org> writes:
> 
> > I have a few questions.
> > 
> > Mark> [ ClassLoader ]
> > Mark>    /**
> > Mark> +   * Normalizes a resource path name. This removes all ./ from the start and
> > Mark> +   * all further '//', "/../" and "/./" from the given name.
> > Mark> +   */
> > 
> > I don't understand why this is a valid operation.  For instance, if
> > the resource comes from the file system, then `..' removal is
> > incorrect in the presence of symlinks.  Maybe it doesn't matter
> > though -- a plausible argument.
> 
> I first wrote some Mauve tests with different params for getResource()
> that should load or not load a particular resource. This was based on
> the directory based loader at first, I then extended it to use different
> URLs to load from. Since the .. and . things didn't work in all cases I
> added this code. But it might not be appropriate when the underlaying
> loader/protocol doesn't support it (like when reading from a jar file).
> I will check how other implementations handle this issue by running
> Mauve against kaffe or Sun implementations. Maybe this should only be
> done in directory based loaders or just let the URL connection handler
> handle it. Will check.

I checked and it is never neccessary to do this. The cases were non
canonical paths are allowed (directory based, but not jar based, file:
and http: URLs) the correct thing is already done automatically by the
underlying protocol. I changed the Mauve tests accordingly and removed
this part of code from the patch (now actually tested the Mauve tests
against the Blackdown 1.4.1 which now also passes all tests.)

> > Mark> +  protected Class findClass(final String className)
> > Mark> +    throws ClassNotFoundException
> > Mark> +  {
> > Mark> +    // Just try to find the resource by the (almost) same name
> > Mark> +    String resourceName = className.replace('.', '/') + ".class";
> > Mark> +    Resource resource = findURLResource(resourceName);
> > 
> > I have a patch here with this hunk:
> > 
> > @@ -266,7 +274,7 @@
> >  
> >      try 
> >        {
> > -	URL url = getResource (name.replace ('.', '/') + ".class");
> > +	URL url = findResource (name.replace ('.', '/') + ".class");
> >  
> >  	if (url == null)
> >  	  throw new ClassNotFoundException (name);
> > 
> > Should findClass be calling findResource instead?  The rationale is
> > that a subclass might override just findResource.  Eclipse does this.
> 
> Yes it should. Thanks for spotting that.
> (getResource would eventually call findResource, but only if none of the
> parent classloaders have the named resource but in that case we
> shouldn't have been called in the first place.)

Now that I have looked closer at the code again I am sure that it should
not call getResource. But calling findResource instead of the private
findURLResource() would make finding/loading the correct class a lot
more difficult and possibly less efficient (findURLResouce returns the
helper class Resource which is then used to do all the Loader specific
Manifest, CodeSource, length and InputStream magic as efficient as
possible for the dir/jar and local/remote cases).

Also there is no mention in any of the documentation that I found that
findClass() will use findResource() to get the class bytes. I did some
tests against Blackdown 1.4.1 and when I subclass URLClassLoader and
only override findResource() then calling findClass() on that
ClassLoader will not call the findResource() method of that class.

Could you show me where/how Eclipse is using this? I would love to
implement it in a way that is compatible with how it is used by Eclipse,
but it might be a bit of work doing so efficiently.

Cheers,

Mark