Spring Security
  1. Spring Security
  2. SEC-1338

Poor concurrency in SessionRegistryImpl

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Complete
    • Affects Version/s: 2.0.5, 3.0.0.RC2
    • Fix Version/s: 3.1.0.M2
    • Component/s: Core
    • Labels:
      None

      Description

      In org.springframework.security.concurrent.SessionRegistryImpl for fields: principals, sessionIds used Collections.synchronizedMap's and for sessionsUsedByPrincipal field used Collections.synchronizedSet.

      Because of it's poor concurrency of synchronized Collections we should used ConcurrentHashMap and CopyOnWriteArraySet instead.

        Activity

        Hide
        Nikita Koksharov added a comment -

        SEC-1338_3.1.0.M2.patch should be applied, usage of CopyOfWriteArraySet replaced by ConcurrentHashMap due low speed of CopyOfWriteArraySet (it creates snapshot of whole collection each time when write occurred) and absence of ConcurrentHashSet implementation in Java 1.5. Only in Java 1.6 method Collections.newSetFromMap(...) exists which could be used to create true concurrent HashSet from ConcurrentHashMap, but it's only in Java 1.6.

        Show
        Nikita Koksharov added a comment - SEC-1338 _3.1.0.M2.patch should be applied, usage of CopyOfWriteArraySet replaced by ConcurrentHashMap due low speed of CopyOfWriteArraySet (it creates snapshot of whole collection each time when write occurred) and absence of ConcurrentHashSet implementation in Java 1.5. Only in Java 1.6 method Collections.newSetFromMap(...) exists which could be used to create true concurrent HashSet from ConcurrentHashMap, but it's only in Java 1.6.
        Hide
        Nikita Koksharov added a comment -

        SEC-1338_3.1.0.M2.patch should be applied to current master version in git.

        Show
        Nikita Koksharov added a comment - SEC-1338 _3.1.0.M2.patch should be applied to current master version in git.
        Hide
        Luke Taylor added a comment -

        Is this really necessary, given normal usage patterns? At the moment, it seems that the use of CopyOnWriteArraySet should not cause a problem. It is used to store the sessions for a principal which should remain small and will not be frequently written to.

        Show
        Luke Taylor added a comment - Is this really necessary, given normal usage patterns? At the moment, it seems that the use of CopyOnWriteArraySet should not cause a problem. It is used to store the sessions for a principal which should remain small and will not be frequently written to.
        Hide
        Nikita Koksharov added a comment -

        I agree, we could keep CopyOnWriteArraySet in usage. You could back issue to "resolve" state

        Show
        Nikita Koksharov added a comment - I agree, we could keep CopyOnWriteArraySet in usage. You could back issue to "resolve" state
        Hide
        Luke Taylor added a comment -

        Ok. Thanks for your input .

        Show
        Luke Taylor added a comment - Ok. Thanks for your input .

          People

          • Assignee:
            Luke Taylor
            Reporter:
            Nikita Koksharov
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: