[GROOVY-3125] Some GroovyClassLoader issues around recompile

• Type: Bug
• Status: Closed
• Priority: Major
• Resolution: Fixed
• Fix Version/s: 1.6.4, 1.7-beta-1
• Assignee: Jochen Theodorou
• Reporter: Michael Fraenkel
• Created: 28/Oct/08 14:16
• Updated: 22/Jul/09 11:02
• Resolved: 22/Jul/09 10:16

Description

After staring at GroovyClassLoader for some bit trying to understand whether one should use loadClass vs parseClass, I ran across a couple of situations where I do not believe the right thing happens. BTW, I do now understand the difference although for my use case, loadClass doesn't fit the bill and it seems GroovyScriptEngine hit the same issues when deal with pure scripts. Anyhow....

This is looking at Groovy 1.5.6

public Class loadClass(final String name, boolean lookupScriptFiles, boolean preferClassOverScript, boolean resolve)
        // look into cache
        Class cls = getClassCacheEntry(name);
        ....
            synchronized (this) {
                // try groovy file
                try {
                    // check if recompilation already happened.
                    final Class classCacheEntry = getClassCacheEntry(name);
                    if (classCacheEntry != cls) return classCacheEntry;
                    URL source = resourceLoader.loadGroovySource(name);
                    cls = recompile(source, name, cls);
                } catch (IOException ioe) {
                    last = new ClassNotFoundException("IOException while opening groovy source: " + name, ioe);
                } finally {
                    if (cls == null) {
                        removeClassCacheEntry(name);
                    } else {
                        setClassCacheEntry(cls);
                    }
                }
            }
        }

The bug here is if recompile fails the classCache is not cleared for this entry. If we had a previous entry, we just put it back. Easy solution is to null out cls before the recompile and pass in the classCacheEntry.

Now for the second issue. When recompile is executed, the sourceCache removes the mapping from script to class, however the parseClass that follows says not to cache the source. If someone did an initial parseClass with a cacheable codeSource, a subsequent loadClass will no longer make that true. Not sure if this is a bug, but its somewhat inconsistent based on which way you deal with Script.

   protected Class recompile(URL source, String className, Class oldClass) 
                ....
                sourceCache.remove(className);
                return parseClass(source.openStream(), className);   <-- creates a non-cacheable CodeSource