Re: svn commit: r156143 - cocoon/trunk/src/java/org/apache/cocoon/generation/JXTemplateGenerator.java cocoon/trunk/status.xml

[Leszek Gawron]

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.

--
Leszek Gawron                                      [EMAIL PROTECTED]
IT Manager                                         MobileBox sp. z o.o.
+48 (61) 855 06 67                              http://www.mobilebox.pl
mobile: +48 (501) 720 812                       fax: +48 (61) 853 29 65