cocoon-cvs mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vgritse...@apache.org
Subject 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/
Date Tue, 18 Mar 2008 19:41:49 GMT
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. 



Mime
View raw message