Cocoon
  1. Cocoon
  2. COCOON-1985

AbstractCachingProcessingPipeline locking with IncludeTransformer may hang pipeline

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1.9, 2.1.10, 2.1.11, 2.2
    • Fix Version/s: 2.1.12, 2.2
    • Component/s: * Cocoon Core
    • Labels:
      None
    • Other Info:
      Patch available
    • Affects version (Component):
      Cocoon Core - 2.2.0-RC2
    • Fix version (Component):
      Cocoon Core - 2.2.0-RC3-dev

      Description

      Cocoon 2.1.9 introduced the concept of a lock in AbstractCachingProcessingPipeline, an optimization to prevent two concurrent requests from generating the same cached content. The first request adds the pipeline key to the transient cache to 'lock' the cache entry for that pipeline, subsequent concurrent requests wait for the first request to cache the content (by Object.lock()ing the pipeline key entry) before proceeding, and can then use the newly cached content.

      However, this has introduced an incompatibility with the IncludeTransformer: if the inclusions access the same yet-to-be-cached content as the root pipeline, the whole assembly hangs, since a lock will be made on a lock already held by the same thread, and which cannot be satisfied.

      e.g.
      i) Root pipeline generates using sub-pipeline cocoon:/foo.xml
      ii) the cocoon:/foo.xml sub-pipeline adds it's pipeline key to the transient store as a lock.
      iii) subsequently in the root pipeline, the IncludeTransformer is run.
      iv) one of the inclusions also generates with cocoon:/foo.xml, this sub-pipeline locks in AbstractProcessingPipeline.waitForLock() because the sub-pipeline key is already present.
      v) deadlock.

      I've found a (partial, see below) solution for this: instead of a plain Object being added to the transient store as the lock object, the Thread.currentThread() is added; when waitForLock() is called, if the lock object exists, it checks that it is not the same thread before attempting to lock it; if it is the same thread, then waitForLock() returns success, which allows generation to proceed. You loose the efficiency of generating the cache only once in this case, but at least it doesn't hang! With JDK1.5 this can be made neater by using Thread#holdsLock() instead of adding the thread object itself to the transient store.

      See patch file.

      However, even with this fix, parallel includes (when enabled) may still hang, because they pass the not-the-same-thread test, but fail because the root pipeline, which holds the initial lock, cannot complete (and therefore statisfy the lock condition for the parallel threads), before the threads themselves have completed, which then results in a deadlock again.

      The complete solution is probably to avoid locking if the lock is held by the same top-level Request, but that requires more knowledge of Cocoon's processing than I (currently) have!

      IMHO unless a complete solution is found to this, then this optimization should be removed completely, or else made optional by configuration, since it renders the IncludeTransformer dangerous.
      1. patch.txt
        1.0 kB
        Ellis Pritchard
      2. sitemap.xmap
        1 kB
        Ellis Pritchard
      3. includer.xsl
        0.3 kB
        Ellis Pritchard
      4. caching-trials.patch
        4 kB
        Alexander Klimetschek
      5. reproduceMultipleThreads.tar.gz
        2 kB
        Alexander Daniel

        Issue Links

          Activity

          Ellis Pritchard created issue -
          Ellis Pritchard made changes -
          Field Original Value New Value
          Attachment patch.txt [ 12349153 ]
          Ellis Pritchard made changes -
          Attachment sitemap.xmap [ 12349186 ]
          Ellis Pritchard made changes -
          Attachment includer.xsl [ 12349187 ]
          Alfred Nathaniel made changes -
          Status Open [ 1 ] On Hold [ 10000 ]
          Alfred Nathaniel made changes -
          Fix Version/s 2.1.11-dev (Current SVN) [ 12312231 ]
          Resolution Fixed [ 1 ]
          Status On Hold [ 10000 ] Closed [ 6 ]
          Ellis Pritchard made changes -
          Fix Version/s 2.1.9 [ 12310650 ]
          Affects Version/s 2.2-dev (Current SVN) [ 12310611 ]
          Fix Version/s 2.1.10 [ 12310931 ]
          Fix Version/s 2.2-dev (Current SVN) [ 12310611 ]
          Sascha-Matthias Kulawik made changes -
          Status Closed [ 6 ] Reopened [ 4 ]
          Resolution Fixed [ 1 ]
          Alexander Klimetschek made changes -
          Attachment caching-trials.patch [ 12352946 ]
          Vadim Gritsenko made changes -
          Fix Version/s 2.1.11-dev (Current SVN) [ 12312231 ]
          Affects version (Component) Parent values: Cocoon Core(10151). Level 1 values: 2.2.0-RC2(10309).
          Fix version (Component) Parent values: Cocoon Core(10227). Level 1 values: 2.2.0-RC3-dev(10304).
          Fix Version/s 2.1.9 [ 12310650 ]
          Fix Version/s 2.1.10 [ 12310931 ]
          Alexander Daniel made changes -
          Attachment reproduceMultipleThreads.tar.gz [ 12377795 ]
          Vadim Gritsenko made changes -
          Fix Version/s 2.1.12-dev (Current SVN) [ 12312903 ]
          Andrew Savory made changes -
          Status Reopened [ 4 ] Closed [ 6 ]
          Resolution Fixed [ 1 ]
          Thorsten Scherler made changes -
          Link This issue is related to COCOON-2241 [ COCOON-2241 ]

            People

            • Assignee:
              Unassigned
              Reporter:
              Ellis Pritchard
            • Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development