You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@cocoon.apache.org by vg...@apache.org on 2008/03/18 20:41:49 UTC
svn commit: r638531 - in /cocoon/branches/BRANCH_2_1_X: ./
src/java/org/apache/cocoon/components/pipeline/impl/
src/webapp/samples/aggregation/ src/webapp/samples/aggregation/content/
Author: vgritsenko
Date: Tue Mar 18 12:41:46 2008
New Revision: 638531
URL: http://svn.apache.org/viewvc?rev=638531&view=rev
Log:
backport fix for COCOON-1985 from trunk:
<action dev="VG" type="fix" fixes-bug="COCOON-1985">
Use current HTTP request as a pipeline lock object instead of
current thread. Resolves deadlock when request is processed
in multiple threads.
</action>
Added:
cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/content/ilock.xml
- copied unchanged from r638388, cocoon/trunk/blocks/cocoon-core-sample/cocoon-core-main-sample/src/main/resources/COB-INF/aggregation/content/ilock.xml
Modified:
cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/samples.xml
cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/sitemap.xmap
cocoon/branches/BRANCH_2_1_X/status.xml
Modified: cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java
URL: http://svn.apache.org/viewvc/cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java?rev=638531&r1=638530&r2=638531&view=diff
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java (original)
+++ cocoon/branches/BRANCH_2_1_X/src/java/org/apache/cocoon/components/pipeline/impl/AbstractCachingProcessingPipeline.java Tue Mar 18 12:41:46 2008
@@ -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.http.HttpEnvironment;
import org.apache.cocoon.transformation.Transformer;
import org.apache.cocoon.util.HashUtil;
import org.apache.excalibur.source.SourceValidity;
@@ -172,106 +173,98 @@
this.readerRole = role;
}
- protected boolean waitForLock(Object key) {
+ protected boolean waitForLock(Environment env, Object key) {
if (transientStore != null) {
- Object lock = null;
+ final String lockKey = PIPELOCK_PREFIX + key;
+
+ // Get a lock object from the store
+ Object lock;
synchronized (transientStore) {
- String lockKey = PIPELOCK_PREFIX + key;
- if (transientStore.containsKey(lockKey)) {
- // cache content is currently being generated, wait for other thread
- lock = transientStore.get(lockKey);
- }
+ lock = transientStore.get(lockKey);
}
// Avoid deadlock with self (see JIRA COCOON-1985).
- if (lock != null && lock != Thread.currentThread()) {
+ Object current = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
+ if (lock != null && lock != current) {
+ if (getLogger().isDebugEnabled()) {
+ getLogger().debug("Waiting on Lock '" + lockKey + "'");
+ }
+
try {
- // become owner of monitor
synchronized (lock) {
lock.wait();
}
- } catch (InterruptedException e) {
+
if (getLogger().isDebugEnabled()) {
- getLogger().debug("Got interrupted waiting for other pipeline to finish processing, retrying...", e);
+ getLogger().debug("Notified on Lock '" + lockKey + "'");
}
- return false;
- }
- if (getLogger().isDebugEnabled()) {
- getLogger().debug("Other pipeline finished processing, retrying to get cached response.");
+ } catch (InterruptedException e) {
+ /* ignored */
}
+
return false;
}
}
+
return true;
}
/**
* makes the lock (instantiates a new object and puts it into the store)
*/
- protected boolean generateLock(Object key) {
- boolean succeeded = true;
-
+ protected void generateLock(Environment env, Object key) {
if (transientStore != null && key != null) {
- String lockKey = PIPELOCK_PREFIX + key;
+ final String lockKey = PIPELOCK_PREFIX + key;
+ if (getLogger().isDebugEnabled()) {
+ getLogger().debug("Adding Lock '" + lockKey + "'");
+ }
+
synchronized (transientStore) {
if (transientStore.containsKey(lockKey)) {
- succeeded = false;
if (getLogger().isDebugEnabled()) {
- getLogger().debug("Lock already present in the store!");
+ getLogger().debug("Lock EXISTS: '" + lockKey + "'");
}
} else {
- Object lock = Thread.currentThread();
+ Object lock = env.getObjectModel().get(HttpEnvironment.HTTP_REQUEST_OBJECT);
try {
transientStore.store(lockKey, lock);
} catch (IOException e) {
- if (getLogger().isDebugEnabled()) {
- getLogger().debug("Could not put lock in the store!", e);
- }
- succeeded = false;
+ /* should not happen */
}
}
}
}
-
- return succeeded;
}
/**
* releases the lock (notifies it and removes it from the store)
*/
- protected boolean releaseLock(Object key) {
- boolean succeeded = true;
-
+ protected void releaseLock(Object key) {
if (transientStore != null && key != null) {
- String lockKey = PIPELOCK_PREFIX + key;
+ final String lockKey = PIPELOCK_PREFIX + key;
+ if (getLogger().isDebugEnabled()) {
+ getLogger().debug("Releasing Lock '" + lockKey + "'");
+ }
+
Object lock = null;
synchronized (transientStore) {
if (!transientStore.containsKey(lockKey)) {
- succeeded = false;
if (getLogger().isDebugEnabled()) {
- getLogger().debug("Lock not present in the store!");
+ getLogger().debug("Lock MISSING: '" + lockKey + "'");
}
} else {
- try {
- lock = transientStore.get(lockKey);
- transientStore.remove(lockKey);
- } catch (Exception e) {
- if (getLogger().isDebugEnabled()) {
- getLogger().debug("Could not get lock from the store!", e);
- }
- succeeded = false;
- }
+ lock = transientStore.get(lockKey);
+ transientStore.remove(lockKey);
}
}
- if (succeeded && lock != null) {
- // become monitor owner
+
+ if (lock != null) {
+ // Notify everybody who's waiting
synchronized (lock) {
lock.notifyAll();
}
}
}
-
- return succeeded;
}
/**
@@ -316,7 +309,7 @@
"' using key " + this.toCacheKey);
}
- generateLock(this.toCacheKey);
+ generateLock(environment, this.toCacheKey);
try {
OutputStream os = null;
@@ -683,9 +676,8 @@
this.completeResponseIsCached = false;
}
} else {
-
// check if there might be one being generated
- if(!waitForLock(this.fromCacheKey)) {
+ if (!waitForLock(environment, this.fromCacheKey)) {
finished = false;
continue;
}
@@ -851,7 +843,7 @@
}
} else {
// check if something is being generated right now
- if(!waitForLock(pcKey)) {
+ if (!waitForLock(environment, pcKey)) {
finished = false;
continue;
}
@@ -867,7 +859,7 @@
getLogger().debug("processReader: caching content for further requests of '" +
environment.getURI() + "'.");
}
- generateLock(pcKey);
+ generateLock(environment, pcKey);
if (readerValidity == null) {
if (isCacheableProcessingComponent) {
Modified: cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/samples.xml
URL: http://svn.apache.org/viewvc/cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/samples.xml?rev=638531&r1=638530&r2=638531&view=diff
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/samples.xml (original)
+++ cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/samples.xml Tue Mar 18 12:41:46 2008
@@ -94,4 +94,12 @@
Example of streaming inclusion using the custom elementpath XPointer scheme.
</sample>
</group>
+
+ <group name="Tests">
+ <sample name="Self Include" href="include-lock">
+ Document created by Cocoon pipeline includes this same pipeline
+ twice, in parallel. Should not cause deadlocks in the caching
+ pipeline (COCOON-1985).
+ </sample>
+ </group>
</samples>
Modified: cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/sitemap.xmap
URL: http://svn.apache.org/viewvc/cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/sitemap.xmap?rev=638531&r1=638530&r2=638531&view=diff
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/sitemap.xmap (original)
+++ cocoon/branches/BRANCH_2_1_X/src/webapp/samples/aggregation/sitemap.xmap Tue Mar 18 12:41:46 2008
@@ -16,8 +16,9 @@
limitations under the License.
-->
-<!-- CVS $Id$ -->
-
+<!--
+ - $Id$
+ -->
<map:sitemap xmlns:map="http://apache.org/cocoon/sitemap/1.0">
<!-- =========================== Views =================================== -->
@@ -194,6 +195,30 @@
<map:parameter name="servletPath" value="{request:servletPath}"/>
<map:parameter name="sitemapURI" value="{request:sitemapURI}"/>
<map:parameter name="file" value="content/test.xml"/>
+ <map:parameter name="remove" value="{0}"/>
+ </map:transform>
+ <map:serialize/>
+ </map:match>
+
+ <!--
+ - COCOON-1985 Pipeline locking and parallel includes:
+ - Generating from cocoon source which includes self, in parallel threads, should not dead lock
+ -->
+ <map:match pattern="lock">
+ <map:generate src="content/ilock.xml"/>
+ <map:serialize type="xml"/>
+ </map:match>
+
+ <map:match pattern="include-lock">
+ <map:generate src="cocoon:/lock"/>
+ <map:transform type="include">
+ <map:parameter name="parallel" value="true"/>
+ </map:transform>
+ <map:transform src="context://samples/common/style/xsl/html/simple-page2html.xsl">
+ <map:parameter name="contextPath" value="{request:contextPath}"/>
+ <map:parameter name="servletPath" value="{request:servletPath}"/>
+ <map:parameter name="sitemapURI" value="{request:sitemapURI}"/>
+ <map:parameter name="file" value="content/ilock.xml"/>
<map:parameter name="remove" value="{0}"/>
</map:transform>
<map:serialize/>
Modified: cocoon/branches/BRANCH_2_1_X/status.xml
URL: http://svn.apache.org/viewvc/cocoon/branches/BRANCH_2_1_X/status.xml?rev=638531&r1=638530&r2=638531&view=diff
==============================================================================
--- cocoon/branches/BRANCH_2_1_X/status.xml (original)
+++ cocoon/branches/BRANCH_2_1_X/status.xml Tue Mar 18 12:41:46 2008
@@ -182,6 +182,11 @@
<changes>
<release version="2.1.12" date="TBD">
+ <action dev="VG" type="fix" fixes-bug="COCOON-1985">
+ Use current HTTP request as a pipeline lock object instead of
+ current thread. Resolves deadlock when request is processed
+ in multiple threads.
+ </action>
<action dev="JH" type="fix">
Core: Close streams properly after copying Parts (MultipartParser). Allow to access InputStream of PartInMemory
multiple times.