Velocity
  1. Velocity
  2. VELOCITY-161

Object reference remains in SimplePool after sucessful get()

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 1.3.1-rc2
    • Fix Version/s: None
    • Component/s: Engine
    • Labels:
      None
    • Environment:
      Operating System: All
      Platform: All

      Description

      When using get() to checkout an object from the
      org.apache.velocity.util.SimplePool, the reference remains in the SimplePool
      and will only get released if the pool grows again to the peak level. The get
      () operation could be easily modified to place a null value in the object
      array location where the pooled item was removed so that if the object is not
      returned to the pool (for whatever reason including exceptions), the reference
      to the object does not remain in the pool until replaced in the stack-based
      design of the SimplePool. It's just good pool management for a reusable class.

      This became obvious when exceptions occured with the VelocityServlet
      mergeTemplate() and objects were not returned to the pool. Memory profiling
      showed references to VelocityWriter objects in the pool that were not released
      until activity increased to replace the reference in the SimplePool object
      array (and the memory could then be reclaimed).

        Activity

        Hide
        Geir Magnusson Jr added a comment -

        Done. Taking out back and shooting now...

        Show
        Geir Magnusson Jr added a comment - Done. Taking out back and shooting now...
        Hide
        Daniel Rall added a comment -

        Geir added this to changes.xml in CVS rev 1.100:

        +<li>
        +SimplePool now removes elements from pool on a get(). Fix for #19042 suggested
        + by Will. (gmj)
        +</li>

        That entry should be modified to describe the semantics change.

        Show
        Daniel Rall added a comment - Geir added this to changes.xml in CVS rev 1.100: +<li> +SimplePool now removes elements from pool on a get(). Fix for #19042 suggested + by Will. (gmj) +</li> That entry should be modified to describe the semantics change.
        Hide
        Daniel Rall added a comment -

        The new semantics sound good to me. An entry should be added to the change log
        for this – I added the new "NeedsReleaseNote" Keyword to indicate that, and
        have tagged the issue with it.

        Show
        Daniel Rall added a comment - The new semantics sound good to me. An entry should be added to the change log for this – I added the new "NeedsReleaseNote" Keyword to indicate that, and have tagged the issue with it.
        Hide
        Geir Magnusson Jr added a comment -

        Fixed and added testcase. This changes the semantics - for now, we can define the behavior as a
        bug, but if someone is depending on it for some reason, we need to change back and just add a
        second class, deprecating the first.

        Show
        Geir Magnusson Jr added a comment - Fixed and added testcase. This changes the semantics - for now, we can define the behavior as a bug, but if someone is depending on it for some reason, we need to change back and just add a second class, deprecating the first.
        Hide
        Will Glass-Husain added a comment -

        Created an attachment (id=8741)
        patch to SimplePool.java (passes ant test)

        Show
        Will Glass-Husain added a comment - Created an attachment (id=8741) patch to SimplePool.java (passes ant test)
        Hide
        Will Glass-Husain added a comment -

        Looked in to this and made a patch. It's not necessary, as all calls to
        SimplePool call the "put" method within a finally block. But it might be
        useful to include as it bullet proofs the code a bit more. (And memory leaks
        are an issue that seem to keep coming up).

        Show
        Will Glass-Husain added a comment - Looked in to this and made a patch. It's not necessary, as all calls to SimplePool call the "put" method within a finally block. But it might be useful to include as it bullet proofs the code a bit more. (And memory leaks are an issue that seem to keep coming up).

          People

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

            Dates

            • Created:
              Updated:
              Resolved:

              Development