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 07:35:15 GMT
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
> >
> > I found that in
> > org 
> > .apache 
> > .cocoon.components.pipeline.impl.AbstractCachingProcessingPipeline  
> > line 245
> > Object lock =
> > env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
> > the lock is null which causes the NPE in the end.
> >
> > I changed it the following way:
> >                     Object lock =
> > env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
> > +                    if (null==lock){
> > +                      lock =
> > env.getObjectModel().get(ObjectModelHelper.REQUEST_OBJECT);
> > +                    }
> >                     try {
> >
> > and now it is working as before.
> >
> > 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,6 +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.ObjectModelHelper;
 import org.apache.cocoon.environment.http.HttpEnvironment;
 import org.apache.cocoon.transformation.Transformer;
 import org.apache.cocoon.util.HashUtil;
@@ -242,7 +243,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. 

Thanks for the feedback Vadim.

salu2

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


Mime
View raw message