Author: bargull
Date: 2007-09-15 12:47:13 -0700 (Sat, 15 Sep 2007)
New Revision: 6496
Modified:
openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzDatapointer.lzs
openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzParsedPath.lzs
Log:
Change 20070915-bargull-2 by [EMAIL PROTECTED] 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
Modified: openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzDatapointer.lzs
===================================================================
--- openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzDatapointer.lzs
2007-09-15 19:44:42 UTC (rev 6495)
+++ openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzDatapointer.lzs
2007-09-15 19:47:13 UTC (rev 6496)
@@ -232,7 +232,8 @@
*/
function xpathQuery ( p ){
var pp = this.parsePath ( p );
- var nodes = this.__LZgetNodes( pp , pp.context ? pp.context : this.p);
+ var ppcontext = pp.getContext(this);
+ var nodes = this.__LZgetNodes( pp , ppcontext ? ppcontext : this.p);
if ( ! nodes ) return null;
if ( pp.aggOperator != null ){
@@ -343,10 +344,11 @@
this.xpath = p;
this.parsedPath = this.parsePath( p );
+ var ppcontext = this.parsedPath.getContext(this);
if ( this.rerunxpath &&
this.parsedPath.hasDotDot &&
- !this.parsedPath.context ){
+ !ppcontext ){
//ruh roh
this.__LZspecialDotDot = true;
} else {
@@ -356,7 +358,7 @@
}
if ( canvas.__LZoldOnData ){
- if ( this.parsedPath.context && ! this.parsedPath.selectors.length &&
+ if ( ppcontext && ! this.parsedPath.selectors.length &&
!this.rerunxpath ){
this.__LZspecialOndata = true;
} else if ( this.__LZspecialOndata ){
@@ -364,7 +366,7 @@
}
}
- this.setDataContext( this.parsedPath.context );
+ this.setDataContext( ppcontext );
return this.runXPath( );
}
@@ -578,13 +580,8 @@
function parsePath ( pa ){
if (pa instanceof LzDatapath) pa = pa.xpath;
var q = this.ppcache[ pa ];
- if ( q ) {
- var l = q['islocaldata']
- if (l) {
- q.context = this.getLocalDataContext(l);
- }
- } else {
- var q = new LzParsedPath( pa, this);
+ if (q == null) {
+ q = new LzParsedPath(pa, this);
this.ppcache[ pa ] = q;
}
return q;
Modified: openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzParsedPath.lzs
===================================================================
--- openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzParsedPath.lzs
2007-09-15 19:44:42 UTC (rev 6495)
+++ openlaszlo/branches/legals/WEB-INF/lps/lfc/data/LzParsedPath.lzs
2007-09-15 19:47:13 UTC (rev 6496)
@@ -13,6 +13,120 @@
*/
class LzParsedPath {
+ /**
+ * The xpath for this LzParsedPath
+ *
+ * @access private
+ */
+ var path = null;
+
+ /**
+ * Array containing selector-operations for this LzParsedPath
+ *
+ * @access private
+ */
+ var selectors = null;
+
+ /**
+ * Pointer to the dataset (if part of the path) otherwise null
+ * (currently only used for "new"-datasets, see deprecated message)
+ *
+ * @access private
+ * @deprecated may lead to memory leaks (LPP-4214)
+ */
+ var context = null;
+
+ /**
+ * Name of the dataset (if part of the path)
+ *
+ * @access private
+ */
+ var dsetname = null;
+
+ /**
+ * Name of the datasource (if part of the path)
+ *
+ * @access private
+ */
+ var dsrcname = null;
+
+ /**
+ * Array containing the path to the local-dataset
+ *
+ * @access private
+ */
+ var islocaldata = null;
+
+ /* Parsing information */
+
+ /**
+ * One of: "nodeName", "__LZgetText", "serialize", "attributes"
+ * or "attributes.xxx" with 'xxx' as an attribute-name
+ *
+ * @access private
+ */
+ var operator = null;
+
+ /**
+ * One of: "position" or "last"
+ *
+ * @access private
+ */
+ var aggOperator = null;
+
+ /**
+ * null or 0
+ *
+ * @access private
+ */
+ var operatorArgs = null;
+
+ /**
+ * true if terminal operator is a attribute-selector, otherwise false
+ *
+ * @access private
+ */
+ var hasAttrOper = false;
+
+ /**
+ * true if path contains a attribute-predicate
+ *
+ * @access private
+ */
+ var hasOpSelector = false;
+
+ /**
+ * true if path contains ".." or "//", otherwise false
+ *
+ * @access private
+ */
+ var hasDotDot = false;
+
+ /**
+ * Return the context for this LzParsedPath
+ * Should be used instead of accessing directly the deprecated field
context.
+ *
+ * @access private
+ */
+ function getContext ( dp ) {
+ if (this.context != null) {
+ return this.context;
+ } else {
+ if (this.islocaldata != null) {
+ return dp.getLocalDataContext(this.islocaldata);
+ } else {
+ if (this.dsrcname != null) {
+ return (canvas[ this.dsrcname ])[ this.dsetname ];
+ } else {
+ if (this.dsetname != null) {
+ return canvas.datasets[ this.dsetname ];
+ }
+ }
+ }
+ }
+ return null;
+ }
+
function initialize ( pa, node ){
//split context part
this.path = pa;
@@ -32,10 +146,11 @@
if (dsrc == 'local') {
nowarn=true;
this.islocaldata = dset.split('.');
- this.context = node.getLocalDataContext(this.islocaldata);
} else {
//we have a datasource and a dataset
- this.context =canvas[ dsrc ] [ dset ];
+ nowarn = ((canvas[ dsrc ])[ dset ] != null);
+ this.dsrcname = dsrc;
+ this.dsetname = dset;
}
} else {
var name = LzParsedPath.trim( sourceparts[ 0 ] );
@@ -48,10 +163,11 @@
this.islocaldata = true;
*/
} else {
- this.context = canvas.datasets[ name ];
+ nowarn = (canvas.datasets[ name ] != null);
+ this.dsetname = name;
}
}
- if ( this.context == null && nowarn != true){
+ if (nowarn != true){
if ($debug) {
Debug.error( "couldn't find dataset for %w", pa );
}
@@ -207,7 +323,6 @@
//this.operator = "t" , "n" , or "a" plus "." and the attribute name
//text, name or attribute, respectively
}
-var operatorArgs = null;
/**
* @access private
_______________________________________________
Laszlo-checkins mailing list
[email protected]
http://www.openlaszlo.org/mailman/listinfo/laszlo-checkins
