[libxslt] Rewrite memory management of local RVTs



commit 470b17346163ba3deceb29eb4149ae140b595cdd
Author: Nick Wellnhofer <wellnhofer aevum de>
Date:   Wed Apr 20 11:50:07 2016 +0200

    Rewrite memory management of local RVTs
    
    The psvi slot of RVTs documents is used to store ownership information.
    
    XSLT_RVT_LOCAL for RVTs that are destroyed after the current instructions
    ends.
    
    XSLT_RVT_VARIABLE for RVTs that are part of a local variable and are
    destroyed after the variable goes out of scope.
    
    XSLT_RVT_FUNC_RESULT for RVTs that are part of results returned with
    func:result. These RVTs won't be destroyed after exiting a template and
    will be reset to XSLT_RVT_LOCAL or XSLT_RVT_VARIABLE in the template
    that receives the return value.
    
    XSLT_RVT_GLOBAL for RVTs that are part of a global variable.
    
    The function xsltFlagRVTs is used for the following ownership
    transitions:
    
    - LOCAL or VARIABLE to FUNC_RESULT when returning a value with
      func:result.
    - FUNC_RESULT to LOCAL or VARIABLE when receiving a func:result.
    - LOCAL to GLOBAL after evaluating global variables or parameters.
    
    This obsoletes the element localRVTBase in the context struct and the
    xsltExtensionInstructionResultRegister function. Aside from the
    func:result implementation, the only reason for the old mechanism was
    to protect RVTs (which can only be returned from extension functions)
    in global variables from being destroyed too early. This is done
    automatically now, so there's no need for extension authors to call
    this function anymore.
    
    The function xsltExtensionInstructionResultFinalize is unsupported
    now. To the best of my knowledge, it isn't used outside of libxslt.
    
    Another benefit is that, in some cases, RVTs are freed earlier now.
    
    Also fixes bug #602531.

 libexslt/common.c                     |    6 -
 libexslt/dynamic.c                    |    3 -
 libexslt/functions.c                  |   23 ++---
 libexslt/strings.c                    |   11 --
 libxslt/transform.c                   |   93 +++++-------------
 libxslt/variables.c                   |  171 +++++++++++++++++++-------------
 libxslt/variables.h                   |    9 ++
 libxslt/xsltInternals.h               |    7 +-
 tests/docs/bug-192.xml                |    1 +
 tests/exslt/functions/Makefile.am     |    3 +-
 tests/exslt/functions/function.11.out |    2 +
 tests/exslt/functions/function.11.xml |    2 +
 tests/exslt/functions/function.11.xsl |   32 ++++++
 tests/general/bug-192.out             |    1 +
 tests/general/bug-192.xsl             |   33 +++++++
 15 files changed, 224 insertions(+), 173 deletions(-)
---
diff --git a/libexslt/common.c b/libexslt/common.c
index 451a60d..91be04f 100644
--- a/libexslt/common.c
+++ b/libexslt/common.c
@@ -60,12 +60,6 @@ exsltNodeSetFunction (xmlXPathParserContextPtr ctxt, int nargs) {
            xsltTransformError(tctxt, NULL, tctxt->inst,
                "exsltNodeSetFunction: Failed to create a node set object.\n");
            tctxt->state = XSLT_STATE_STOPPED;
-       } else {
-           /*
-            * Mark it as a function result in order to avoid garbage
-            * collecting of tree fragments
-            */
-           xsltExtensionInstructionResultRegister(tctxt, obj);
        }
        if (strval != NULL)
            xmlFree (strval);
diff --git a/libexslt/dynamic.c b/libexslt/dynamic.c
index 7b95fc5..4a4944e 100644
--- a/libexslt/dynamic.c
+++ b/libexslt/dynamic.c
@@ -216,7 +216,6 @@ exsltDynMapFunction(xmlXPathParserContextPtr ctxt, int nargs)
                                 xmlXPathNodeSetAddUnique(ret->nodesetval,
                                                          cur);
                             }
-                           xsltExtensionInstructionResultRegister(tctxt, ret);
                         }
                         break;
                     case XPATH_NUMBER:
@@ -239,7 +238,6 @@ exsltDynMapFunction(xmlXPathParserContextPtr ctxt, int nargs)
                                 xmlXPathNodeSetAddUnique(ret->nodesetval,
                                                          cur);
                             }
-                           xsltExtensionInstructionResultRegister(tctxt, ret);
                         }
                         break;
                     case XPATH_STRING:
@@ -257,7 +255,6 @@ exsltDynMapFunction(xmlXPathParserContextPtr ctxt, int nargs)
                                 xmlXPathNodeSetAddUnique(ret->nodesetval,
                                                          cur);
                             }
-                           xsltExtensionInstructionResultRegister(tctxt, ret);
                         }
                         break;
                    default:
diff --git a/libexslt/functions.c b/libexslt/functions.c
index 0795a13..b2c98d5 100644
--- a/libexslt/functions.c
+++ b/libexslt/functions.c
@@ -35,7 +35,6 @@ struct _exsltFuncData {
     xmlHashTablePtr funcs;     /* pointer to the stylesheet module data */
     xmlXPathObjectPtr result;  /* returned by func:result */
     int error;                 /* did an error occur? */
-    xmlDocPtr RVT;   /* result tree fragment */
 };
 
 typedef struct _exsltFuncResultPreComp exsltFuncResultPreComp;
@@ -428,6 +427,12 @@ exsltFuncFunctionFunction (xmlXPathParserContextPtr ctxt, int nargs) {
 
     if (data->result != NULL) {
        ret = data->result;
+        /*
+        * IMPORTANT: This enables previously tree fragments marked as
+        * being results of a function, to be garbage-collected after
+        * the calling process exits.
+        */
+        xsltFlagRVTs(tctxt, ret, XSLT_RVT_LOCAL);
     } else
        ret = xmlXPathNewCString("");
 
@@ -452,12 +457,6 @@ exsltFuncFunctionFunction (xmlXPathParserContextPtr ctxt, int nargs) {
     valuePush(ctxt, ret);
 
 error:
-    /*
-    * IMPORTANT: This enables previously tree fragments marked as
-    * being results of a function, to be garbage-collected after
-    * the calling process exits.
-    */
-    xsltExtensionInstructionResultFinalize(tctxt);
     tctxt->funcLevel--;
 }
 
@@ -724,7 +723,7 @@ exsltFuncResultElem (xsltTransformContextPtr ctxt,
        * Mark it as a function result in order to avoid garbage
        * collecting of tree fragments before the function exits.
        */
-       xsltExtensionInstructionResultRegister(ctxt, ret);
+       xsltFlagRVTs(ctxt, ret, XSLT_RVT_FUNC_RESULT);
     } else if (inst->children != NULL) {
        /* If the func:result element does not have a select attribute
         * and has non-empty content (i.e. the func:result element has
@@ -741,7 +740,8 @@ exsltFuncResultElem (xsltTransformContextPtr ctxt,
            data->error = 1;
            return;
        }
-       xsltRegisterLocalRVT(ctxt, container);
+        /* Mark as function result. */
+        container->psvi = XSLT_RVT_FUNC_RESULT;
 
        oldInsert = ctxt->insert;
        ctxt->insert = (xmlNodePtr) container;
@@ -756,11 +756,6 @@ exsltFuncResultElem (xsltTransformContextPtr ctxt,
            data->error = 1;
        } else {
            ret->boolval = 0; /* Freeing is not handled there anymore */
-           /*
-           * Mark it as a function result in order to avoid garbage
-           * collecting of tree fragments before the function exits.
-           */
-           xsltExtensionInstructionResultRegister(ctxt, ret);
        }
     } else {
        /* If the func:result element has empty content and does not
diff --git a/libexslt/strings.c b/libexslt/strings.c
index f5f2d3c..62f76fb 100644
--- a/libexslt/strings.c
+++ b/libexslt/strings.c
@@ -111,11 +111,6 @@ exsltStrTokenizeFunction(xmlXPathParserContextPtr ctxt, int nargs)
                 xmlAddChild((xmlNodePtr) container, node);
                xmlXPathNodeSetAddUnique(ret->nodesetval, node);
             }
-           /*
-            * Mark it as a function result in order to avoid garbage
-            * collecting of tree fragments
-            */
-           xsltExtensionInstructionResultRegister(tctxt, ret);
         }
     }
 
@@ -222,11 +217,6 @@ exsltStrSplitFunction(xmlXPathParserContextPtr ctxt, int nargs) {
                xmlAddChild((xmlNodePtr) container, node);
                xmlXPathNodeSetAddUnique(ret->nodesetval, node);
            }
-           /*
-            * Mark it as a function result in order to avoid garbage
-            * collecting of tree fragments
-            */
-           xsltExtensionInstructionResultRegister(tctxt, ret);
         }
     }
 
@@ -543,7 +533,6 @@ exsltStrReturnString(xmlXPathParserContextPtr ctxt, const xmlChar *str,
         return(-1);
     }
 
-    xsltExtensionInstructionResultRegister(tctxt, ret);
     valuePush(ctxt, ret);
 
     return(0);
diff --git a/libxslt/transform.c b/libxslt/transform.c
index 8b86e2e..9faaede 100644
--- a/libxslt/transform.c
+++ b/libxslt/transform.c
@@ -56,6 +56,7 @@
 #ifdef WITH_XSLT_DEBUG
 #define WITH_XSLT_DEBUG_EXTRA
 #define WITH_XSLT_DEBUG_PROCESS
+#define WITH_XSLT_DEBUG_VARIABLE
 #endif
 
 #define XSLT_GENERATE_HTML_DOCTYPE
@@ -2299,30 +2300,28 @@ xsltReleaseLocalRVTs(xsltTransformContextPtr ctxt, xmlDocPtr base)
 {
     xmlDocPtr cur = ctxt->localRVT, tmp;
 
-    while ((cur != NULL) && (cur != base)) {
-       if (cur->psvi == (void *) ((long) 1)) {
-           cur = (xmlDocPtr) cur->next;
-       } else {
-           tmp = cur;
-           cur = (xmlDocPtr) cur->next;
-
-           if (tmp == ctxt->localRVT)
-               ctxt->localRVT = cur;
+    if (cur == base)
+        return;
+    if (cur->prev != NULL)
+        xsltTransformError(ctxt, NULL, NULL, "localRVT not head of list\n");
 
-           /*
-           * We need ctxt->localRVTBase for extension instructions
-           * which return values (like EXSLT's function).
-           */
-           if (tmp == ctxt->localRVTBase)
-               ctxt->localRVTBase = cur;
+    do {
+        tmp = cur;
+        cur = (xmlDocPtr) cur->next;
+        if (tmp->psvi == XSLT_RVT_LOCAL) {
+            xsltReleaseRVT(ctxt, tmp);
+        } else if (tmp->psvi == XSLT_RVT_GLOBAL) {
+            xsltRegisterPersistRVT(ctxt, tmp);
+        } else if (tmp->psvi != XSLT_RVT_FUNC_RESULT) {
+            xmlGenericError(xmlGenericErrorContext,
+                    "xsltReleaseLocalRVTs: Unexpected RVT flag %p\n",
+                    tmp->psvi);
+        }
+    } while (cur != base);
 
-           if (tmp->prev)
-               tmp->prev->next = (xmlNodePtr) cur;
-           if (cur)
-               cur->prev = tmp->prev;
-           xsltReleaseRVT(ctxt, tmp);
-       }
-    }
+    if (base != NULL)
+        base->prev = NULL;
+    ctxt->localRVT = base;
 }
 
 /**
@@ -2348,7 +2347,7 @@ xsltApplySequenceConstructor(xsltTransformContextPtr ctxt,
     xmlNodePtr oldInsert, oldInst, oldCurInst, oldContextNode;
     xmlNodePtr cur, insert, copy = NULL;
     int level = 0, oldVarsNr;
-    xmlDocPtr oldLocalFragmentTop, oldLocalFragmentBase;
+    xmlDocPtr oldLocalFragmentTop;
 
 #ifdef XSLT_REFACTORED
     xsltStylePreCompPtr info;
@@ -2676,16 +2675,9 @@ xsltApplySequenceConstructor(xsltTransformContextPtr ctxt,
                        cur->name));
 #endif
                    ctxt->insert = insert;
-                   /*
-                   * We need the fragment base for extension instructions
-                   * which return values (like EXSLT's function).
-                   */
-                   oldLocalFragmentBase = ctxt->localRVTBase;
-                   ctxt->localRVTBase = NULL;
 
                    func(ctxt, contextNode, cur, cur->psvi);
 
-                   ctxt->localRVTBase = oldLocalFragmentBase;
                    /*
                    * Cleanup temporary tree fragments.
                    */
@@ -2749,12 +2741,9 @@ xsltApplySequenceConstructor(xsltTransformContextPtr ctxt,
                oldCurInst = ctxt->inst;
                ctxt->inst = cur;
                 ctxt->insert = insert;
-               oldLocalFragmentBase = ctxt->localRVTBase;
-               ctxt->localRVTBase = NULL;
 
                 info->func(ctxt, contextNode, cur, (xsltElemPreCompPtr) info);
 
-               ctxt->localRVTBase = oldLocalFragmentBase;
                /*
                * Cleanup temporary tree fragments.
                */
@@ -2869,12 +2858,6 @@ xsltApplySequenceConstructor(xsltTransformContextPtr ctxt,
 #endif
 
                 ctxt->insert = insert;
-               /*
-               * We need the fragment base for extension instructions
-               * which return values (like EXSLT's function).
-               */
-               oldLocalFragmentBase = ctxt->localRVTBase;
-               ctxt->localRVTBase = NULL;
 
                 function(ctxt, contextNode, cur, cur->psvi);
                /*
@@ -2883,7 +2866,6 @@ xsltApplySequenceConstructor(xsltTransformContextPtr ctxt,
                if (oldLocalFragmentTop != ctxt->localRVT)
                    xsltReleaseLocalRVTs(ctxt, oldLocalFragmentTop);
 
-               ctxt->localRVTBase = oldLocalFragmentBase;
                 ctxt->insert = oldInsert;
 
             }
@@ -3059,7 +3041,7 @@ xsltApplyXSLTTemplate(xsltTransformContextPtr ctxt,
     long start = 0;
     xmlNodePtr cur;
     xsltStackElemPtr tmpParam = NULL;
-    xmlDocPtr oldUserFragmentTop, oldLocalFragmentTop;
+    xmlDocPtr oldUserFragmentTop;
 
 #ifdef XSLT_REFACTORED
     xsltStyleItemParamPtr iparam;
@@ -3122,7 +3104,6 @@ xsltApplyXSLTTemplate(xsltTransformContextPtr ctxt,
 
     oldUserFragmentTop = ctxt->tmpRVT;
     ctxt->tmpRVT = NULL;
-    oldLocalFragmentTop = ctxt->localRVT;
 
     /*
     * Initiate a distinct scope of local params/variables.
@@ -3224,31 +3205,6 @@ xsltApplyXSLTTemplate(xsltTransformContextPtr ctxt,
     ctxt->varsBase = oldVarsBase;
 
     /*
-    * Clean up remaining local tree fragments.
-    * This also frees fragments which are the result of
-    * extension instructions. Should normally not be hit; but
-    * just for the case xsltExtensionInstructionResultFinalize()
-    * was not called by the extension author.
-    */
-    if (oldLocalFragmentTop != ctxt->localRVT) {
-       xmlDocPtr curdoc = ctxt->localRVT, tmp;
-
-       do {
-           tmp = curdoc;
-           curdoc = (xmlDocPtr) curdoc->next;
-           /* Need to housekeep localRVTBase */
-           if (tmp == ctxt->localRVTBase)
-               ctxt->localRVTBase = curdoc;
-           if (tmp->prev)
-               tmp->prev->next = (xmlNodePtr) curdoc;
-           if (curdoc)
-               curdoc->prev = tmp->prev;
-           xsltReleaseRVT(ctxt, tmp);
-       } while (curdoc != oldLocalFragmentTop);
-    }
-    ctxt->localRVT = oldLocalFragmentTop;
-
-    /*
     * Release user-created fragments stored in the scope
     * of xsl:template. Note that this mechanism is deprecated:
     * user code should now use xsltRegisterLocalRVT() instead
@@ -6040,6 +5996,9 @@ xsltApplyStylesheetInternal(xsltStylesheetPtr style, xmlDocPtr doc,
 
     xsltEvalGlobalVariables(ctxt);
 
+    /* Clean up any unused RVTs. */
+    xsltReleaseLocalRVTs(ctxt, NULL);
+
     ctxt->node = (xmlNodePtr) doc;
     ctxt->output = res;
     ctxt->insert = (xmlNodePtr) res;
diff --git a/libxslt/variables.c b/libxslt/variables.c
index 345123d..52cd68d 100644
--- a/libxslt/variables.c
+++ b/libxslt/variables.c
@@ -43,8 +43,8 @@ const xmlChar *xsltDocFragFake = (const xmlChar *) " fake node libxslt";
 const xmlChar *xsltComputingGlobalVarMarker =
  (const xmlChar *) " var/param being computed";
 
-#define XSLT_VAR_GLOBAL 1<<0
-#define XSLT_VAR_IN_SELECT 1<<1
+#define XSLT_VAR_GLOBAL (1<<0)
+#define XSLT_VAR_IN_SELECT (1<<1)
 #define XSLT_TCTXT_VARIABLE(c) ((xsltStackElemPtr) (c)->contextVariable)
 
 /************************************************************************
@@ -122,6 +122,9 @@ xsltRegisterTmpRVT(xsltTransformContextPtr ctxt, xmlDocPtr RVT)
     if ((ctxt == NULL) || (RVT == NULL))
        return(-1);
 
+    RVT->prev = NULL;
+    RVT->psvi = XSLT_RVT_VARIABLE;
+
     /*
     * We'll restrict the lifetime of user-created fragments
     * insinde an xsl:variable and xsl:param to the lifetime of the
@@ -159,15 +162,18 @@ xsltRegisterLocalRVT(xsltTransformContextPtr ctxt,
     if ((ctxt == NULL) || (RVT == NULL))
        return(-1);
 
+    RVT->prev = NULL;
+
     /*
     * When evaluating "select" expressions of xsl:variable
     * and xsl:param, we need to bind newly created tree fragments
-    * to the variable itself; otherwise the tragment will be
+    * to the variable itself; otherwise the fragment will be
     * freed before we leave the scope of a var.
     */
     if ((ctxt->contextVariable != NULL) &&
        (XSLT_TCTXT_VARIABLE(ctxt)->flags & XSLT_VAR_IN_SELECT))
     {
+        RVT->psvi = XSLT_RVT_VARIABLE;
        RVT->next = (xmlNodePtr) XSLT_TCTXT_VARIABLE(ctxt)->fragment;
        XSLT_TCTXT_VARIABLE(ctxt)->fragment = RVT;
        return(0);
@@ -177,19 +183,11 @@ xsltRegisterLocalRVT(xsltTransformContextPtr ctxt,
     * If not reference by a returning instruction (like EXSLT's function),
     * then this fragment will be freed, when the instruction exits.
     */
+    RVT->psvi = XSLT_RVT_LOCAL;
     RVT->next = (xmlNodePtr) ctxt->localRVT;
     if (ctxt->localRVT != NULL)
        ctxt->localRVT->prev = (xmlNodePtr) RVT;
     ctxt->localRVT = RVT;
-    /*
-    * We need to keep track of the first registered fragment
-    * for extension instructions which return fragments
-    * (e.g. EXSLT'S function), in order to let
-    * xsltExtensionInstructionResultFinalize() clear the
-    * preserving flag on the fragments.
-    */
-    if (ctxt->localRVTBase == NULL)
-       ctxt->localRVTBase = RVT;
     return(0);
 }
 
@@ -204,26 +202,16 @@ xsltRegisterLocalRVT(xsltTransformContextPtr ctxt,
  * collector will free them after the function-calling process exits.
  *
  * Returns 0 in case of success and -1 in case of API or internal errors.
+ *
+ * This function is unsupported in newer releases of libxslt.
  */
 int
 xsltExtensionInstructionResultFinalize(xsltTransformContextPtr ctxt)
 {
-    xmlDocPtr cur;
-
-    if (ctxt == NULL)
-       return(-1);
-    if (ctxt->localRVTBase == NULL)
-       return(0);
-    /*
-    * Enable remaining local tree fragments to be freed
-    * by the fragment garbage collector.
-    */
-    cur = ctxt->localRVTBase;
-    do {
-       cur->psvi = NULL;
-       cur = (xmlDocPtr) cur->next;
-    } while (cur != NULL);
-    return(0);
+    xmlGenericError(xmlGenericErrorContext,
+            "xsltExtensionInstructionResultFinalize is unsupported "
+            "in this release of libxslt.\n");
+    return(-1);
 }
 
 /**
@@ -238,11 +226,35 @@ xsltExtensionInstructionResultFinalize(xsltTransformContextPtr ctxt)
  * tree fragments (via xsltCreateRVT()) with xsltRegisterLocalRVT().
  *
  * Returns 0 in case of success and -1 in case of error.
+ *
+ * It isn't necessary to call this function in newer releases of
+ * libxslt.
  */
 int
 xsltExtensionInstructionResultRegister(xsltTransformContextPtr ctxt,
                                       xmlXPathObjectPtr obj)
 {
+    return(0);
+}
+
+/**
+ * xsltFlagRVTs:
+ * @ctxt: an XSLT transformation context
+ * @obj: an XPath object to be inspected for result tree fragments
+ * @val: the flag value
+ *
+ * Updates ownership information of RVTs in @obj according to @val.
+ *
+ * @val = XSLT_RVT_FUNC_RESULT for the result of an extension function, so its
+ *        RVTs won't be destroyed after leaving the returning scope.
+ * @val = XSLT_RVT_LOCAL for the result of an extension function to reset
+ *        the state of its RVTs after it was returned to a new scope.
+ * @val = XSLT_RVT_GLOBAL for parts of global variables.
+ *
+ * Returns 0 in case of success and -1 in case of error.
+ */
+int
+xsltFlagRVTs(xsltTransformContextPtr ctxt, xmlXPathObjectPtr obj, void *val) {
     int i;
     xmlNodePtr cur;
     xmlDocPtr doc;
@@ -275,36 +287,59 @@ xsltExtensionInstructionResultRegister(xsltTransformContextPtr ctxt,
                doc = cur->doc;
            } else {
                xsltTransformError(ctxt, NULL, ctxt->inst,
-                   "Internal error in "
-                   "xsltExtensionInstructionResultRegister(): "
+                   "Internal error in xsltFlagRVTs(): "
                    "Cannot retrieve the doc of a namespace node.\n");
-               goto error;
+               return(-1);
            }
        } else {
            doc = cur->doc;
        }
        if (doc == NULL) {
            xsltTransformError(ctxt, NULL, ctxt->inst,
-               "Internal error in "
-               "xsltExtensionInstructionResultRegister(): "
+               "Internal error in xsltFlagRVTs(): "
                "Cannot retrieve the doc of a node.\n");
-           goto error;
+           return(-1);
        }
-       if (doc->name && (doc->name[0] == ' ')) {
+       if (doc->name && (doc->name[0] == ' ') &&
+            doc->psvi != XSLT_RVT_GLOBAL) {
            /*
            * This is a result tree fragment.
-           * We'll use the @psvi field for reference counting.
-           * TODO: How do we know if this is a value of a
-           *  global variable or a doc acquired via the
+           * We store ownership information in the @psvi field.
+           * TODO: How do we know if this is a doc acquired via the
            *  document() function?
            */
-           doc->psvi = (void *) ((long) 1);
+#ifdef WITH_XSLT_DEBUG_VARIABLE
+            XSLT_TRACE(ctxt,XSLT_TRACE_VARIABLES,xsltGenericDebug(xsltGenericDebugContext,
+                "Flagging RVT %p: %p -> %p\n", doc, doc->psvi, val));
+#endif
+
+            if (val == XSLT_RVT_LOCAL) {
+                if (doc->psvi != XSLT_RVT_FUNC_RESULT) {
+                   xmlGenericError(xmlGenericErrorContext,
+                            "xsltFlagRVTs: Invalid transition %p => LOCAL\n",
+                            doc->psvi);
+                    return(-1);
+                }
+
+                xsltRegisterLocalRVT(ctxt, doc);
+            } else if (val == XSLT_RVT_GLOBAL) {
+                if (doc->psvi != XSLT_RVT_LOCAL) {
+                   xmlGenericError(xmlGenericErrorContext,
+                            "xsltFlagRVTs: Invalid transition %p => GLOBAL\n",
+                            doc->psvi);
+                    doc->psvi = XSLT_RVT_GLOBAL;
+                    return(-1);
+                }
+
+                /* Will be registered as persistant in xsltReleaseLocalRVTs. */
+                doc->psvi = XSLT_RVT_GLOBAL;
+            } else if (val == XSLT_RVT_FUNC_RESULT) {
+               doc->psvi = val;
+            }
        }
     }
 
     return(0);
-error:
-    return(-1);
 }
 
 /**
@@ -350,9 +385,9 @@ xsltReleaseRVT(xsltTransformContextPtr ctxt, xmlDocPtr RVT)
        }
 
        /*
-       * Reset the reference counter.
+       * Reset the ownership information.
        */
-       RVT->psvi = 0;
+       RVT->psvi = NULL;
 
        RVT->next = (xmlNodePtr) ctxt->cache->RVT;
        ctxt->cache->RVT = RVT;
@@ -391,6 +426,8 @@ xsltRegisterPersistRVT(xsltTransformContextPtr ctxt, xmlDocPtr RVT)
 {
     if ((ctxt == NULL) || (RVT == NULL)) return(-1);
 
+    RVT->psvi = XSLT_RVT_GLOBAL;
+    RVT->prev = NULL;
     RVT->next = (xmlNodePtr) ctxt->persistRVT;
     if (ctxt->persistRVT != NULL)
        ctxt->persistRVT->prev = (xmlNodePtr) RVT;
@@ -541,35 +578,21 @@ xsltFreeStackElem(xsltStackElemPtr elem) {
     /*
     * Release the list of temporary Result Tree Fragments.
     */
-    if (elem->fragment) {
+    if (elem->context) {
        xmlDocPtr cur;
 
        while (elem->fragment != NULL) {
            cur = elem->fragment;
            elem->fragment = (xmlDocPtr) cur->next;
 
-           if (elem->context &&
-               (cur->psvi == (void *) ((long) 1)))
-           {
-               /*
-               * This fragment is a result of an extension instruction
-               * (e.g. XSLT's function) and needs to be preserved until
-               * the instruction exits.
-               * Example: The fragment of the variable must not be freed
-               *  since it is returned by the EXSLT function:
-               *  <f:function name="foo">
-               *   <xsl:variable name="bar">
-               *     <bar/>
-               *   </xsl:variable>
-               *   <f:result select="$bar"/>
-               *  </f:function>
-               *
-               */
-               xsltRegisterLocalRVT(elem->context, cur);
-           } else {
+            if (cur->psvi == XSLT_RVT_VARIABLE) {
                xsltReleaseRVT((xsltTransformContextPtr) elem->context,
                    cur);
-           }
+            } else if (cur->psvi != XSLT_RVT_FUNC_RESULT) {
+                xmlGenericError(xmlGenericErrorContext,
+                        "xsltFreeStackElem: Unexpected RVT flag %p\n",
+                        cur->psvi);
+            }
        }
     }
     /*
@@ -963,6 +986,7 @@ xsltEvalVariable(xsltTransformContextPtr ctxt, xsltStackElemPtr variable,
                * the Result Tree Fragment.
                */
                variable->fragment = container;
+                container->psvi = XSLT_RVT_VARIABLE;
 
                oldOutput = ctxt->output;
                oldInsert = ctxt->insert;
@@ -1149,16 +1173,23 @@ xsltEvalGlobalVariable(xsltStackElemPtr elem, xsltTransformContextPtr ctxt)
                xsltTransformError(ctxt, NULL, comp->inst,
                    "Evaluating global variable %s failed\n", elem->name);
            ctxt->state = XSLT_STATE_STOPPED;
+            goto error;
+        }
+
+        /*
+         * Mark all RVTs that are referenced from result as part
+         * of this variable so they won't be freed too early.
+         */
+        xsltFlagRVTs(ctxt, result, XSLT_RVT_GLOBAL);
+
 #ifdef WITH_XSLT_DEBUG_VARIABLE
 #ifdef LIBXML_DEBUG_ENABLED
-       } else {
-           if ((xsltGenericDebugContext == stdout) ||
-               (xsltGenericDebugContext == stderr))
-               xmlXPathDebugDumpObject((FILE *)xsltGenericDebugContext,
-                                       result, 0);
+       if ((xsltGenericDebugContext == stdout) ||
+           (xsltGenericDebugContext == stderr))
+           xmlXPathDebugDumpObject((FILE *)xsltGenericDebugContext,
+                                   result, 0);
 #endif
 #endif
-       }
     } else {
        if (elem->tree == NULL) {
            result = xmlXPathNewCString("");
diff --git a/libxslt/variables.h b/libxslt/variables.h
index 17b4c6f..f80eeab 100644
--- a/libxslt/variables.h
+++ b/libxslt/variables.h
@@ -35,6 +35,15 @@ extern "C" {
     (ctxt)->xpathCtxt->extra = ctxt
 
 /*
+ * Flags for memory management of RVTs
+ */
+
+#define XSLT_RVT_LOCAL       ((void *)1)
+#define XSLT_RVT_VARIABLE    ((void *)2)
+#define XSLT_RVT_FUNC_RESULT ((void *)3)
+#define XSLT_RVT_GLOBAL      ((void *)4)
+
+/*
  * Interfaces for the variable module.
  */
 
diff --git a/libxslt/xsltInternals.h b/libxslt/xsltInternals.h
index 7123ace..b56cbdd 100644
--- a/libxslt/xsltInternals.h
+++ b/libxslt/xsltInternals.h
@@ -1783,7 +1783,7 @@ struct _xsltTransformContext {
     xmlDocPtr localRVT; /* list of local tree fragments; will be freed when
                           the instruction which created the fragment
                            exits */
-    xmlDocPtr localRVTBase;
+    xmlDocPtr localRVTBase; /* Obsolete */
     int keyInitLevel;   /* Needed to catch recursive keys issues */
     int funcLevel;      /* Needed to catch recursive functions issues */
     int maxTemplateDepth;
@@ -1906,6 +1906,11 @@ XSLTPUBFUN int XSLTCALL
 XSLTPUBFUN int XSLTCALL
                        xsltExtensionInstructionResultFinalize(
                                                 xsltTransformContextPtr ctxt);
+XSLTPUBFUN int XSLTCALL
+                       xsltFlagRVTs(
+                                                xsltTransformContextPtr ctxt,
+                                                xmlXPathObjectPtr obj,
+                                                void *val);
 XSLTPUBFUN void XSLTCALL
                        xsltFreeRVTs            (xsltTransformContextPtr ctxt);
 XSLTPUBFUN void XSLTCALL
diff --git a/tests/docs/bug-192.xml b/tests/docs/bug-192.xml
new file mode 100644
index 0000000..69d62f2
--- /dev/null
+++ b/tests/docs/bug-192.xml
@@ -0,0 +1 @@
+<doc/>
diff --git a/tests/exslt/functions/Makefile.am b/tests/exslt/functions/Makefile.am
index 303043a..3241284 100644
--- a/tests/exslt/functions/Makefile.am
+++ b/tests/exslt/functions/Makefile.am
@@ -13,7 +13,8 @@ EXTRA_DIST =                                          \
        function.7.out  function.7.xml  function.7.xsl  \
        function.8.out  function.8.xml  function.8.xsl  \
        function.9.out  function.9.xml  function.9.xsl  \
-       function.10.out function.10.xml function.10.xsl
+       function.10.out function.10.xml function.10.xsl \
+       function.11.out function.11.xml function.11.xsl
 
 CLEANFILES = .memdump
 
diff --git a/tests/exslt/functions/function.11.out b/tests/exslt/functions/function.11.out
new file mode 100644
index 0000000..6dde03e
--- /dev/null
+++ b/tests/exslt/functions/function.11.out
@@ -0,0 +1,2 @@
+<?xml version="1.0"?>
+<a><b/></a>
diff --git a/tests/exslt/functions/function.11.xml b/tests/exslt/functions/function.11.xml
new file mode 100644
index 0000000..8e39ecb
--- /dev/null
+++ b/tests/exslt/functions/function.11.xml
@@ -0,0 +1,2 @@
+<?xml version="1.0"?>
+<doc/>
diff --git a/tests/exslt/functions/function.11.xsl b/tests/exslt/functions/function.11.xsl
new file mode 100644
index 0000000..7a3437a
--- /dev/null
+++ b/tests/exslt/functions/function.11.xsl
@@ -0,0 +1,32 @@
+<?xml version="1.0"?>
+
+<xsl:stylesheet
+    version="1.0"
+    xmlns:xsl="http://www.w3.org/1999/XSL/Transform";
+    xmlns:func="http://exslt.org/functions";
+    xmlns:test="test"
+    extension-element-prefixes="func"
+>
+    <func:function name="test:fragment">
+        <func:result>
+            <a>
+                <b/>
+            </a>
+        </func:result>
+    </func:function>
+
+    <func:function name="test:func1">
+        <xsl:variable name="var" select="test:fragment()"/>
+        <func:result select="$var"/>
+    </func:function>
+
+    <func:function name="test:func2">
+        <xsl:variable name="var" select="test:func1()"/>
+        <func:result select="$var"/>
+    </func:function>
+
+    <xsl:template match="/">
+        <xsl:copy-of select="test:func2()"/>
+    </xsl:template>
+</xsl:stylesheet>
+
diff --git a/tests/general/bug-192.out b/tests/general/bug-192.out
new file mode 100644
index 0000000..6c2eeca
--- /dev/null
+++ b/tests/general/bug-192.out
@@ -0,0 +1 @@
+assert 'success!' == 'success!'
diff --git a/tests/general/bug-192.xsl b/tests/general/bug-192.xsl
new file mode 100644
index 0000000..1307211
--- /dev/null
+++ b/tests/general/bug-192.xsl
@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<xsl:stylesheet version="1.0"
+  xmlns:xsl='http://www.w3.org/1999/XSL/Transform'
+  xmlns:func='http://exslt.org/functions'
+  xmlns:foo='http://foo.com/'
+  extension-element-prefixes='func'
+ >
+<xsl:output method="text" />
+
+<!-- $func-value should have the value 'success!' but instead reports an 
+empty string -->
+<xsl:variable name="func-value" select="foo:get-value()" />
+<xsl:variable name="dummy-value" select="$func-value" />
+<xsl:variable name="template-value">
+   <xsl:call-template name="get-dummy" />
+</xsl:variable>
+
+<func:function name="foo:get-value">
+   <func:result>success!</func:result>
+</func:function>
+
+<xsl:template name="get-dummy">
+   <xsl:value-of select="$dummy-value" />
+</xsl:template>
+
+<xsl:template match="/">
+   <xsl:text>assert 'success!' == '</xsl:text>
+   <xsl:value-of select="$func-value" />
+   <xsl:text>'
+</xsl:text>
+</xsl:template>
+
+</xsl:stylesheet>


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]