[#LPP-4214] "LzDatapointer#ppcache" leaks memory

OpenLaszlo

"LzDatapointer#ppcache" leaks memory

Created: 28/Jun/07 03:17 PM Updated: 27/Sep/07 08:55 AM

Component/s:

LFC - Data

Affects Version/s:

4.0.2

Fix Version/s:

RingDing (4.1)

Time Tracking:

Not Specified

File Attachments:

LPP-4214.lzx (5 kb)

Severity:	Minor
Fixed in Change#:	6,496
Runtime:	N/A
Flags:	External
Fix in hand:	False

Description

« Hide

"LzParsedPath" holds a reference to the associated dataset in its field "context".
As every (no limitation -> memory consumption?) "LzParsedPath" gets cached by "LzDatapointer" in "ppcache", and as this cache won't ever be updated in any manner (besides adding new entries), this leads to a memory leak as soon as the associated dataset will be destroyed, because the cached "LzParsedPath" still references to the already destroyed dataset.

Additional info:
This caching-behaviour (of never deleting an entry), breaks xpath-Queries if a developer ever wants to create/destroy datasets on the fly. After the initially created dataset has been destroyed and a new dataset with the same name has been generated, the cached LzParsedPath still references to the first (destroyed) dataset in its context-field, and no to the newly created dataset.

Proposals:
- removing the "ppcache"-entry, when the associated datasets gets destroyed
- alternatively, nullify the "context"-field on the cached "LzParsedPath", so you can save the parsing-operations stored in the "LzParsedPath"-object, but the memory won't any longer leak. If ever a new dataset with the same name will be created, you just have to update the "context"-field. (Just take the latter alternative if caching is really that important, 'cause how high is the probability of: "same dataset-name" + "same xpath-Query"...?)
- limitation on the "ppcache" to prevent unlimited caching of LzParsedPath-objects

All

Comments

Work Log

Change History

Sort Order:

Ascending order - Click to sort in descending order

[ Permlink | « Hide ]

P T Withington - 03/Jul/07 01:57 PM

Have a look at the other caches in the system. We seem to have a penchant for adding them without considering how they will be managed...

[ Permlink | « Hide ]

André Bargull - 04/Sep/07 05:03 PM

When this bug is fixed, you can also close "~~LPP-363~~".

[ Permlink | « Hide ]

André Bargull - 06/Sep/07 04:03 PM

Right now I'm working on a patch for this issue. I guess I'll send it out in a day or two for review.

[ Permlink | « Hide ]

André Bargull - 09/Sep/07 03:08 AM

Attached testcase "~~LPP-4241~~.lzx"

[ Permlink | « Hide ]

André Bargull - 09/Sep/07 03:10 AM

Should be:
Attached testcase "~~LPP-4214~~.lzx"

[ Permlink | « Hide ]

André Bargull - 15/Sep/07 12:49 PM

r6496 | bargull | 2007-09-15 21:47:13 +0200 (Sat, 15 Sep 2007) | 36 lines
Ge?\195?\164nderte Pfade:
   M /openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzDatapointer.lzs
   M /openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzParsedPath.lzs

Change 20070915-bargull-2 by bargull@dell--p4--2-53 on 2007-09-15 21:39:09
    in /home/Admin/src/svn/openlaszlo/branches/legals
    for http://svn.openlaszlo.org/openlaszlo/branches/legals

Summary: Fix for memory leak in LzParsedPath

New Features:

Bugs Fixed:
~~LPP-4214~~ - "LzDatapointer#ppcache" leaks memory

Technical Reviewer: hminsky
QA Reviewer: ptw
Doc Reviewer: (pending)

Documentation:
Added "getContext()" to LzParsedPath which replaces the direct access to "context"-member of "LzParsedPath".
With this change the "context"-member of LzParsedPath is only used for "new"-datasets (xpath:"new:/foo/bar").
This API-Change was necessary, because LzParsedPath was holding a reference to a dataset through his "context"-member,
but even if this dataset was destroyed, the reference was not cleared.
This led to two bugs:
1. it was preventing garbage-collection
2. when a user created a new dataset with the same name, cached LzParsedPaths were still pointing to the old dataset,
    which gave some strange errors i.e. when a user used xpath:"ds:/foo/text()" (cached) this gave the old results,
    but xpath:"ds:/foo" (non-cached) and then a LzDatapointer#getNodeText() gave new results.
    For better understanding of this issue, please see the attached testcase at ~~LPP-4214~~.

Release Notes:

Details:
The marked memory leaks in the testcase are covered by LPP-4688

Tests:
Testcase is attached at ~~LPP-4214~~