Vadim Gritsenko wrote:
[EMAIL PROTECTED] wrote:
Author: lgawron Date: Fri Mar 4 00:39:53 2005 New Revision: 156143
URL: http://svn.apache.org/viewcvs?view=rev&rev=156143
Log:
Fix thread safety problem in JXTemplateGenerator.setup() concerning template script reparsing.
Modified: cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java
URL: http://svn.apache.org/viewcvs/cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java?view=diff&r1=156142&r2=156143
==============================================================================
--- cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java (original)
+++ cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java Fri Mar 4 00:39:53 2005
@@ -2357,7 +2357,6 @@
valid = startEvent.compileTime.isValid(validity);
}
if (valid != SourceValidity.VALID) {
- cache.remove(uri);
regenerate = true;
}
} else {
What good this does? Second thread, instead of quick fail on first test (if (startEvent != null)), will go through complete validity check. And it will arrive to the same result, that it need to re-generate from source. So, it will arrive to the same SourceUtil.parse line.
PS I'd mark the bug INVALID as I do not see what's broken. Yes, it can parse it twice. It's not an error by itself. Wrapping whole parsing block into the synchronized() is abviously not a solution too, especially for larger sites.
Maybe I had it wrong but imagine the situation with two threads: Thread1 Thread2 1) JXTG.setup() got script, script is valid JXTG.setup() finish
2) JXTG.setup() script invalid so removed from cache
3) JXTG.generate() script = cache.get(...) script is null as second thread removed it while it should still be there because it was valid at the setup() stage
4) script reparsed JXTG.setup() finish()
This is not a race condition between two JXTG.setup() but with JXTG.generate() in first thread and JXTG.setup() in second one.
Ok, got it. In this case proper solution is not to use cache in the generate, but obtain startEvent instance in the setup and save it for generate in member variable. In this case, generate method won't have a chance of accessing modified (in this case - removed) data.
Also, private static cache map should go in favor of the store component. Otherwise, larger site might simply run out of memory 'cause JXTemplateGenerator ate all of it.
Vadim
