Commons Dbcp
  1. Commons Dbcp
  2. DBCP-320

.PerUserPoolDataSource.poolKeys can grow without limit

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.2.2
    • Fix Version/s: 1.4
    • Labels:
      None

      Description

      The private static variable .PerUserPoolDataSource.poolKeys (a HashMap) only ever has items added to it; the contents are never cleared.

      Every different datasource generates a new HashMap entry; every username+password generates a new entry in that map.

      This might perhaps become a problem for long-running applications with lots of different users.

        Activity

        Hide
        Phil Steitz added a comment -

        Fixed in r900226

        Show
        Phil Steitz added a comment - Fixed in r900226
        Hide
        Phil Steitz added a comment - - edited

        I agree - looks to me as though poolKeys can safely be eliminated. It appears to be a global cache for UserKey objects, reused across PerUserPoolDataSource instances and to have no purpose other than caching the PoolKeys. It is private and used only in getPoolKey.

        Show
        Phil Steitz added a comment - - edited I agree - looks to me as though poolKeys can safely be eliminated. It appears to be a global cache for UserKey objects, reused across PerUserPoolDataSource instances and to have no purpose other than caching the PoolKeys. It is private and used only in getPoolKey.
        Hide
        Sebb added a comment -

        As far as I can make out, the Map is being used as a cache for PoolKey objects.

        As these are immutable and relatively small, it seems to me that caching them serves little purpose - the class might as well create the PoolKey each time.

        If caching is necessary, then surely it would be better to use an instance variable to hold the cache, and make the cache an LRUMap, as is done for SharedPoolDataSource?
        [although that class has even less need for a cache!]

        Show
        Sebb added a comment - As far as I can make out, the Map is being used as a cache for PoolKey objects. As these are immutable and relatively small, it seems to me that caching them serves little purpose - the class might as well create the PoolKey each time. If caching is necessary, then surely it would be better to use an instance variable to hold the cache, and make the cache an LRUMap, as is done for SharedPoolDataSource? [although that class has even less need for a cache!]

          People

          • Assignee:
            Unassigned
            Reporter:
            Sebb
          • Votes:
            0 Vote for this issue
            Watchers:
            0 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development