cocoon-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Thorsten Scherler <thorsten.scherler....@juntadeandalucia.es>
Subject Re: [2.1] AbstractCachingProcessingPipeline and cocoon CLI
Date Fri, 22 Aug 2008 08:49:52 GMT
On Fri, 2008-08-22 at 09:35 +0200, Thorsten Scherler wrote:
> On Thu, 2008-08-21 at 19:40 -0400, Vadim Gritsenko wrote:
> > On Aug 21, 2008, at 8:18 AM, Thorsten Scherler wrote:
> > 
> > > still updating forrest to use cocoon-2.1.x and I found a problem in  
> > > the
> > > AbstractCachingProcessingPipeline.
> > >
> > > I am not sure whether someone is using the cocoon cli ATM. Forrest is
> > > based around this component.
> > >
> > > https://issues.apache.org/jira/browse/FOR-955?focusedCommentId=12624340 
> > > #action_12624340
...
> > > Can somebody verify that it is a bug? Should I commit the patch?
> > 
> > It is a bug, but with proposed change you can get a deadlock under  
> > some conditions (IIRC, when using parallels inclusion in the sitemap  
> > which ultimately pull from the same pipeline). The idea behind that  
> > lock is to synchronize on the main request (top most request), and not  
> > on any of sub-request object created during aggregation. Suitable  
> > alternative would be to lock against top most command line request.
> 
> How about:

Index:
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
===================================================================
---
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
(revision 681296)
+++
src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
(working copy)
@@ -36,7 +36,7 @@
 import org.apache.cocoon.caching.ComponentCacheKey;
 import org.apache.cocoon.caching.PipelineCacheKey;
 import org.apache.cocoon.environment.Environment;
-import org.apache.cocoon.environment.http.HttpEnvironment;
+import org.apache.cocoon.environment.ObjectModelHelper;
 import org.apache.cocoon.transformation.Transformer;
 import org.apache.cocoon.util.HashUtil;
 import org.apache.excalibur.source.SourceValidity;
@@ -192,7 +192,7 @@
             }
 
             // Avoid deadlock with self (see JIRA COCOON-1985).
-            Object current =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+            Object current =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
             if (lock != null && lock != current) {
                 if (getLogger().isDebugEnabled()) {
                     getLogger().debug("Waiting on Lock '" + lockKey +
"'");
@@ -242,7 +242,7 @@
                         getLogger().debug("Lock EXISTS: '" + lockKey +
"'");
                     }
                 } else {
-                    Object lock =
env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+                    Object lock =
env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
                     try {
                         transientStore.store(lockKey, lock);
                     } catch (IOException e) {

This way we always lock the same object. I had a look as well on COCOON-1985 and it should
be compatible.

Thanks for the feedback Vadim.

salu2
-- 
Thorsten Scherler                                 thorsten.at.apache.org
Open Source Java                      consulting, training and solutions


Mime
View raw message