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
