Spring 2.5.X refresh interval incompatibility

Have you tried with Spring 3?
Spring 3 was upgraded to Groovy 1.7, AFAIK, but not Spring 2.5, still based on Groovy 1.6.

More on Spring 3...
We have to be careful: if ever we fix the issue you mention, we shouldn't break Spring 3's Groovy support!!!

I will try that on Spring 3.0 and will let you know. But for several companies (such as ours) upgrading to Spring 3.0 is not an option because of JDK15 requirements or necessity to update own libraries to adapt Spring 3.0 changes, which takes some time (depending on how tight integration was).

I still hope, that Groovy will still support JDK14 though artifact groovy-all-jdk14-1.6.5 was damaged and not fully usable in older environments. Or will it not? I noticed that 1.7.0 port for JDK14 is still missing on Maven repositories ...

Refresh fails even with Spring 3.0.0.RELEASE from repo1/maven2 repository. I include zip with tests using new release of the Spring Framework and the test still fails. Is it even possible with the test coverage that is standard for Spring products? Isn't it a problem of my local environment? I hope not.

Reuploaded ZIPs - there were dependencies on our company parent pom. These poms should be universal (I hope).

Corrected pom.xml files again so you can run it without problems - removed problematic log4j transitive dependencies and configured compiler plugin.

Anyone knows which spring/groovy components are involved in spring's <lang:groovy> support? If I have to look a bit into this issue where groovy bean refresh is having some issue, which groovy components should I keep an eye on?

GROOVY-3980 was clearly entering into groovy through GroovyScriptEngine, but in this one entry point from spring into groovy is not clear to me.

Can anyone help with that please?

This is duplicate of GROOVY-3973.

Even in this JIRA, the call is coming from spring's GroovyScriptFactory into GroovyClassLoader.parseClass(String, String) - exact same issue as reported in GROOVY-3973.

As far as I know there is class org.springframework.scripting.groovy.GroovyScriptFactory that takes care of creating Groovy classes. It uses GroovyClassLoader to do that - so no GroovyScriptengine is used there.

In general Spring should generate for each Groovy bean dynamic proxy - I think that it is always JdkDynamicAopProxy in case of Groovy bean. This proxy then delegates method call on its TargetSource, that is represented by RefreshableScriptTargetSource. This implementation asks GroovyScriptFactory to check whether Groovy instance needs refresh, but only in specified intervals that are defined on the <lang:groovy/> tag of the particular bean in Spring context (in our case 0).

GroovyScriptFactory consults ScriptSource.isModified() and when this object returns true, new Groovy instance should be created through via freshTarget() method.

What confuses me mosts that it seems that problem is not caused by the Groovy itself. Even if file gets changed in the test's modifyGroovyFile method, the:

public final synchronized Object getTarget() {
if ((refreshCheckDelayElapsed() && requiresRefresh()) || this.targetObject == null)

return this.targetObject;
}

in the AbstractRefreshableTarget source returns:

true for refreshCheckDelayElapsed - ok, because there is zero set on the bean, so check should be triggered every time
false for requiresRefresh - and this is the problem, bud I cannot undestand why it still returns fals even if file timestamp changed

Moreover all those classes are Spring classes - so why this work with old Groovy and not the new version, when it seems that Groovy doesn't get involved in all of this (whole obsolete checking is happening in Spring classes)?

I am still looking at that ... maybe I found something, but I doubt.

Yes, no GroovyScriptEngine in this case. The calls come from GroovyScriptFactory into GroovyClassLoader.

I was about to start tracing Spring code from ClassPathXmlApplicationContext -> GroovyScriptFactory -> GroovyClassLoader, and I am sure the details you have provided will help me understand that much better. Thanks a lot.

I am pretty sure that it is duplicate of GROOVY-3973 and the fix for GROOVY-3633 has affected your case too. I am still looking into it. May be I will try reverting the fix of GROOVY-3633 and see if your test start going through.

I will write back as soon as I have more details to add.

Yes, if I just revert the fix of GROOVY-3633, your test starts going through. So, the origin of the issue and that it is duplicate of GROOVY-3973 is confirmed now.

I think fix for GROOVY-3633 has to be changed.

Hi Jochen,
I wanted to propose changing the current fix of GROOVY-3633 and am attaching an alternative patch.

The issue in GROOVY-3633(2nd part) was that if a cacheable GroovyCodeSource gets compiled and later recompile() is called on it, it removes the entry of that GroovyCodeSource from sourceCache and the recompiled class never gets cached again as the new GroovyCodeSource always has cacheable flag set as false.

In the proposed patch, when we cache the class for a cacheable GroovyCodeSource, we make note of that in a separate cache and use that later in recompile() when it makes the new GroovyCodeSource - instead of having "false", it will again have "true", as needed by GROOVY-3633 - but done in a different way - and not hardcoding it "true" always.

With this change, this regression on the spring bean refresh goes away.

What do you think?

so basically the problem is that a String does not get compiled again, because the source name is reused and the old one was cached. While I agree that making String sources cacheable by default might have been the wrong decision I don't like that map. You need it because after recompilation the information of being cacheable would be lost else? what happens if you disable caching for the string based version by default?

Here is another patch that just makes caching disabled in parseClass(String, String).

This change is also helping with Spring reloading as Spring's GroovyScriptFactory only uses GCL#parseClass(String, String).

So, as far as regression with Spring integration is concerned, even this patch v2 helps as much as the previous one.

I would like to hear Guillaume on this. I think that I can approve of this patch now. Looks much better than the map version.

Ok, let's wait for Guillaume's inputs on it. I am not applying the patch right now.

Hi Jan, regarding testing reloading of groovy beans through spring, I have a suggestion. I think your reload tests need to change a bit to see consistent results.

Spring calls GroovyClassLoader to re-parse the class only when RefreshableScriptTargetSource#requiresRefresh() returns true. This is based on ResourceScriptSource#isModified(), which compares the previous lastModified timestamp of the groovy script file with the current timestamp.

Now in your tests, you first call original MainGroovyClass#sayHello(), then you modify the file and then call it again to check if Groovy is re-parsing the script. But you have many tests that modify the same file to test the reload the script file and intermittently it is possible that the timestamps it sees before and after modification are in the same millisecond value - and, intermittently, in those cases, Spring will not invoke GCL#parseClass() and reloading will not happen and the test will fail.

So, the suggestion is that before you modify the script file during a test, you can introduce sleep of a few milli-seconds - to force the pre and post modification timestamps to have different values.

What do you think?

The previous comment is all about what happens within Spring's reloading/refresh logic. If following happen too fast (in the same milli-second), then Spring itself doesn't call groovy parseClass and then groovy cannot do anything :

1) getBean(aGroovyScript) call 1
2) modify the groovy script configured in spring context
1) getBean(aGroovyScript) call 2

Yes Roshan, that's a good idea. I will alter the tests with adding a small waiting section before modifyFile. I tried to play with that, but because it didn't help with the issue I removed that again. But this may be the purpose why some of the tests fail on our company integration server, that is probably much more fast than my local machine where all test pass with 1.6.5 Groovy.

After applying the fix being discussed here, when I ran the failing test (of this JIRA) individually, it runs successfully every single time, but when I run all 16 tests from your code together - intermittently one reload test will fail or another (my desktop is fast too ). Some digging up showed that both pre-post file modification, timestamps were coming as same. A delay in between should help there.

I've modified the tests so that there is a second wait time. Now even on the fastest machines the timestamp must change . Just in case you would like to test it again, Roshan.

How long do you guess will take to get those bugfixes into the GA release? Which one will it be - 1.7.1?

Yes, Jan, the bug fixes will come out only in 1.7.1, which, I have heard from Guillaume, will happen within a month or so.

In the meanwhile, you have the option to use 1.7.1 snapshot versions.

Please note that fix for this JIRA (being done by changing GROOVY-3633 fix) is not in the codebase yet, as Jochen wanted Guillaume also to review it.

Fix for GROOVY-3980 is already checked-in and is available in the snapshot.

@Jan - this fix is in the 1.7.x codebase now (against GROOVY-3973) and should get released in 1.7.1.

I tested with your new tests (with the delay) and they seem to be consistently passing now.

@Jochen - I checked with Guillaume before checking-in the fix.

Thank you Roshan - even my internal integration server test fail issue was corrected by the advice you gave me. Looking forward to 1.7.1