groovy

Some GroovyClassLoader issues around recompile

Details

  • Type: Bug Bug
  • Status: Closed Closed
  • Priority: Major Major
  • Resolution: Fixed
  • Affects Version/s: None
  • Fix Version/s: 1.6.4, 1.7-beta-1
  • Component/s: None
  • Labels:
    None
  • Number of attachments :
    0

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

Activity

Ascending order - Click to sort in descending order
Guillaume Laforge made changes -
Field Original Value New Value
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
 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
{code}
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);
                    }
                }
            }
        }
{code}
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.
{code}
   protected Class recompile(URL source, String className, Class oldClass)
                ....
                sourceCache.remove(className);
                return parseClass(source.openStream(), className); <-- creates a non-cacheable CodeSource
{code}
blackdrag blackdrag made changes -
Fix Version/s 1.7-beta-1 [ 14014 ]
blackdrag blackdrag made changes -
Resolution Fixed [ 1 ]
Fix Version/s 1.6.4 [ 15291 ]
Assignee Jochen Theodorou [ blackdrag ]
Status Open [ 1 ] Closed [ 6 ]

People

  • Assignee:
    blackdrag blackdrag
    Reporter:
    Michael Fraenkel
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: