Tapestry
  1. Tapestry
  2. TAPESTRY-1571

CheckForUpdatesFilter can cause deadlock

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 5.0.5
    • Fix Version/s: 5.0.5
    • Component/s: tapestry-core
    • Labels:
      None

      Description

      CheckForUpdatesFilter will cause a deadlock in the following circumstances.
      1) Initial request is received and processing starts (CheckForUpdatesFilter read lock obtained)
      2) Request processing triggers another http request to the same application
      3) Nested request is received
      4) CheckForUpdatesFilter determines it is time to check for file updates
      5) CheckForUpdatesFilter tries to obtain write lock
      6) Deadlock: Nested request is blocked waiting for read lock held by parent request to be released.

      This is an unusual situation, generally I wouldn't try to to access a page via http whilst trying to process another request. However I ran across this problem whilst using a third party library (JasperReports) that referenced resources via http links.

      Although uncommon, I believe Tapestry should be able to handle this without deadlocking

      1. TAP-1571.patch
        17 kB
        Ben Sommerville

        Activity

        Ben Sommerville created issue -
        Hide
        Ben Sommerville added a comment -

        I can think of a couple of approaches for solving this.
        a) put a timeout on the request for a write lock.
        b) define a flag/parameter in a request/url that will disable checking for updates.

        (a) seems the better approach to me. In fact, given that checking is primarily for development (where load will be very light), it would probably work to turn the write lock request into a tryLock, & continue straight away if another thread has the read lock.

        Show
        Ben Sommerville added a comment - I can think of a couple of approaches for solving this. a) put a timeout on the request for a write lock. b) define a flag/parameter in a request/url that will disable checking for updates. (a) seems the better approach to me. In fact, given that checking is primarily for development (where load will be very light), it would probably work to turn the write lock request into a tryLock, & continue straight away if another thread has the read lock.
        Hide
        Ben Sommerville added a comment -

        Patch that:

        • adds a tryWithWrite method to ConcurrentBarrier
        • changes CheckForUpdatesFilter to use tryWithWrite instead of withWrite
        • add configution parameter for the try update timeout.
        Show
        Ben Sommerville added a comment - Patch that: adds a tryWithWrite method to ConcurrentBarrier changes CheckForUpdatesFilter to use tryWithWrite instead of withWrite add configution parameter for the try update timeout.
        Ben Sommerville made changes -
        Field Original Value New Value
        Attachment TAP-1571.patch [ 12359673 ]
        Hide
        Ben Sommerville added a comment -

        The patch I just added fixes the deadlock problem in my application.
        Dynamic reloading of pages still works & I haven't seen any problems yet.

        I will continue to test & update this issue if needed.

        Show
        Ben Sommerville added a comment - The patch I just added fixes the deadlock problem in my application. Dynamic reloading of pages still works & I haven't seen any problems yet. I will continue to test & update this issue if needed.
        Hide
        Howard M. Lewis Ship added a comment -

        I'm curious ... what are you doing that clearing out the cache causes a loopback request into your application?

        I'll likely take the patch either way (deadlocks are verboten, no matter what), but I want to know if there's an alternative.

        Show
        Howard M. Lewis Ship added a comment - I'm curious ... what are you doing that clearing out the cache causes a loopback request into your application? I'll likely take the patch either way (deadlocks are verboten, no matter what), but I want to know if there's an alternative.
        Hide
        Ben Sommerville added a comment -

        The loopback is not caused by clearing the cache. The problem shows up when a cache clear is triggered by a loopback request.

        Specifically in my case I am using Jasper Reports to produce some PDF reports for the user. The app has a page where the user can request a report to be run & then see the PDF result. So the creation of the report happens completely within the context of that page request.

        The reports that are being produced include images and other sub reports. These other resources are external to the main report definition. The main report includes a reference which is ultimately resolved to an URL (its more complicated in practice but that is the net effect).

        When Jasper is processing the main report it hits one of the image and/or sub-report references & fetches that image/subreport from the specified URL This request is done by the Jasper engine within the context of the initial page request.

        It is these requests (by Jasper) for report resources which trigger a deadlock. This occurs when one of those requests is received by the app & the CheckForUpdatesFilter determines it is time to carry out an update check.

        The filter tries to obtain its write lock (line 85) but it can't because the initial page request holds a read lock. The initial request never relinquishes the read lock because it waiting for the resource request to return. (That is probably not strictly true... I expect something would timeout eventually).

        Ideally Jasper would have a more pluggable mechanism for resolving references to external resources. However it has no extension points at all so I am left with supplying an URL back to the application.

        I hope that all makes sense.

        Show
        Ben Sommerville added a comment - The loopback is not caused by clearing the cache. The problem shows up when a cache clear is triggered by a loopback request. Specifically in my case I am using Jasper Reports to produce some PDF reports for the user. The app has a page where the user can request a report to be run & then see the PDF result. So the creation of the report happens completely within the context of that page request. The reports that are being produced include images and other sub reports. These other resources are external to the main report definition. The main report includes a reference which is ultimately resolved to an URL (its more complicated in practice but that is the net effect). When Jasper is processing the main report it hits one of the image and/or sub-report references & fetches that image/subreport from the specified URL This request is done by the Jasper engine within the context of the initial page request. It is these requests (by Jasper) for report resources which trigger a deadlock. This occurs when one of those requests is received by the app & the CheckForUpdatesFilter determines it is time to carry out an update check. The filter tries to obtain its write lock (line 85) but it can't because the initial page request holds a read lock. The initial request never relinquishes the read lock because it waiting for the resource request to return. (That is probably not strictly true... I expect something would timeout eventually). Ideally Jasper would have a more pluggable mechanism for resolving references to external resources. However it has no extension points at all so I am left with supplying an URL back to the application. I hope that all makes sense.
        Hide
        Howard M. Lewis Ship added a comment -

        Nice patch, and I think a reasonable timeout on getting the write lock is appropriate for an under-load server even outside the context of deadlock's.

        We may find that we need to escalate the timeout over a series of events, or we may get to a point where we are unable (under load) to check for updates at all.

        Show
        Howard M. Lewis Ship added a comment - Nice patch, and I think a reasonable timeout on getting the write lock is appropriate for an under-load server even outside the context of deadlock's. We may find that we need to escalate the timeout over a series of events, or we may get to a point where we are unable (under load) to check for updates at all.
        Howard M. Lewis Ship made changes -
        Fix Version/s 5.0.5 [ 12312477 ]
        Resolution Fixed [ 1 ]
        Assignee Howard M. Lewis Ship [ hlship ]
        Status Open [ 1 ] Closed [ 6 ]
        Hide
        Ben Sommerville added a comment -

        FYI - I just discovered the other day that due to a JVM bug, this patch will cause a deadlock when running under jdk 6u1. The JVM bug report is at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6571733. Apparently it is fixed under jdk 6u2 & jdk7 so it is not a big issue.

        If tapestry has to work under jdk6u1 then changing the ConcurrentBarrier read/write lock to "fair" mode will get rid of the deadlock.

        Show
        Ben Sommerville added a comment - FYI - I just discovered the other day that due to a JVM bug, this patch will cause a deadlock when running under jdk 6u1. The JVM bug report is at http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=6571733 . Apparently it is fixed under jdk 6u2 & jdk7 so it is not a big issue. If tapestry has to work under jdk6u1 then changing the ConcurrentBarrier read/write lock to "fair" mode will get rid of the deadlock.
        Mark Thomas made changes -
        Workflow jira [ 12406212 ] Default workflow, editable Closed status [ 12568209 ]
        Mark Thomas made changes -
        Workflow Default workflow, editable Closed status [ 12568209 ] jira [ 12591287 ]

          People

          • Assignee:
            Howard M. Lewis Ship
            Reporter:
            Ben Sommerville
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development