Jetspeed 2
  1. Jetspeed 2
  2. JS2-589

PermissionManagerImpl use of ThreadLocal for caching causes inconsistent results

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Critical Critical
    • Resolution: Fixed
    • Affects Version/s: 2.1-dev
    • Fix Version/s: 2.2.2
    • Component/s: Security
    • Labels:
      None
    • Environment:
      java 1.5, oracle, any OS

      Description

      PermissionManagerImpl uses ThreadLocal for permission caching. This means that once the permissions are loaded for one thread (ex. thread 1), they are there until someone changes the permissions using the PermissionManager (on that thread). If permissions are edited on another thread (thread 2), thread 1 never gets updated. So permissions are inconsistent until the portal is restarted.

        Activity

        Hide
        David Sean Taylor added a comment -

        So the threads are being reused by the application server. I assumed that the app server would clear the thread local. Guess not.
        The goal for the caching was to reuse permissions on a per request basis. Would it help if we cleared the thread local cache at the end of the request pipeline?
        Its either that or find a way to sync up the thread local caches...

        Show
        David Sean Taylor added a comment - So the threads are being reused by the application server. I assumed that the app server would clear the thread local. Guess not. The goal for the caching was to reuse permissions on a per request basis. Would it help if we cleared the thread local cache at the end of the request pipeline? Its either that or find a way to sync up the thread local caches...
        Hide
        Ethan Adams added a comment -

        I think clearing the ThreadLocal value through the pipeline is a good option if you only want per request caching. On the other hand, it seems this component and many others (page manager, portlet entity access, etc. ) would benefit from a consistent cache model through some kind of caching service. You could use an existing cache project for the backing to a generic cache service.

        Just a thought.

        Show
        Ethan Adams added a comment - I think clearing the ThreadLocal value through the pipeline is a good option if you only want per request caching. On the other hand, it seems this component and many others (page manager, portlet entity access, etc. ) would benefit from a consistent cache model through some kind of caching service. You could use an existing cache project for the backing to a generic cache service. Just a thought.
        Hide
        David Sean Taylor added a comment -

        I couldn't agree more with your comments on a consistent cache model.
        I'll take that approach in the long run. Do you need a more immediate fix, such as the clearing of the thread local in the pipeline ?

        Show
        David Sean Taylor added a comment - I couldn't agree more with your comments on a consistent cache model. I'll take that approach in the long run. Do you need a more immediate fix, such as the clearing of the thread local in the pipeline ?
        Hide
        Ethan Adams added a comment -

        I agree that clearing the ThreadLocal through the pipeline is the best immediate fix.

        However, I'd hate to see a clear() method as part of the PermissionManager interface. I suppose it could be done by calling implementation specific methods via casting.

        Again...just my 2 cents.

        Show
        Ethan Adams added a comment - I agree that clearing the ThreadLocal through the pipeline is the best immediate fix. However, I'd hate to see a clear() method as part of the PermissionManager interface. I suppose it could be done by calling implementation specific methods via casting. Again...just my 2 cents.
        Hide
        Ate Douma added a comment -

        I think we should either should leverage transparent JPA caching (once that's replacing OJB) or else add separate ehcache solution.
        Targeting to 2.2.1 release

        Show
        Ate Douma added a comment - I think we should either should leverage transparent JPA caching (once that's replacing OJB) or else add separate ehcache solution. Targeting to 2.2.1 release
        Hide
        David Sean Taylor added a comment -

        We won't be addressing this in 2.2.1

        Show
        David Sean Taylor added a comment - We won't be addressing this in 2.2.1
        Hide
        Ate Douma added a comment -

        I'll implement the ThreadLocal cleanup on ServletRequest using the new ServletRequestCleanupService from JS2-1253

        Show
        Ate Douma added a comment - I'll implement the ThreadLocal cleanup on ServletRequest using the new ServletRequestCleanupService from JS2-1253

          People

          • Assignee:
            Ate Douma
            Reporter:
            Ethan Adams
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development