Memory leak in higher-order functions

extensions/exquery/restxq/src/main/java/org/exist/extensions/exquery/restxq/impl/ResourceFunctionExecutorImpl.java

@@ -161,25 +161,26 @@ public Sequence execute(final ResourceFunction resourceFunction, final Iterable<
             processMonitor.queryStarted(xqueryContext.getWatchDog());
 
             //create a function call
-            final FunctionReference fnRef = new FunctionReference(new FunctionCall(xqueryContext, fn));
-
-            //convert the arguments
-            final org.exist.xquery.value.Sequence[] fnArgs = convertToExistFunctionArguments(xqueryContext, fn, arguments);
-
-            //execute the function call
-            fnRef.analyze(new AnalyzeContextInfo());
-
-            //if setUid/setGid, determine the effectiveSubject to use for execution
-            final Optional<EffectiveSubject> effectiveSubject = getEffectiveSubject(xquery);
-
-            try {
-                effectiveSubject.ifPresent(broker::pushSubject);  //switch to effective user if setUid/setGid
-                final org.exist.xquery.value.Sequence result = fnRef.evalFunction(null, null, fnArgs);
-                return new SequenceAdapter(result);
-            } finally {
-                //switch back from effective user if setUid/setGid
-                if(effectiveSubject.isPresent()) {
-                    broker.popSubject();
+            try (final FunctionReference fnRef = new FunctionReference(new FunctionCall(xqueryContext, fn))) {
+
+                //convert the arguments
+                final org.exist.xquery.value.Sequence[] fnArgs = convertToExistFunctionArguments(xqueryContext, fn, arguments);
+
+                //execute the function call
+                fnRef.analyze(new AnalyzeContextInfo());
+
+                //if setUid/setGid, determine the effectiveSubject to use for execution
+                final Optional<EffectiveSubject> effectiveSubject = getEffectiveSubject(xquery);
+
+                try {
+                    effectiveSubject.ifPresent(broker::pushSubject);  //switch to effective user if setUid/setGid
+                    final org.exist.xquery.value.Sequence result = fnRef.evalFunction(null, null, fnArgs);
+                    return new SequenceAdapter(result);
+                } finally {
+                    //switch back from effective user if setUid/setGid
+                    if (effectiveSubject.isPresent()) {
+                        broker.popSubject();
+                    }
                 }
             }
 

extensions/indexes/ngram/src/org/exist/xquery/modules/ngram/HighlightMatches.java

@@ -78,41 +78,42 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) throws XPathExce
         if (args[0].isEmpty())
             return Sequence.EMPTY_SEQUENCE;
 
-        FunctionReference func = (FunctionReference) args[1].itemAt(0);
+        try (FunctionReference func = (FunctionReference) args[1].itemAt(0)) {
 
-        NGramIndexWorker index = (NGramIndexWorker) context.getBroker().getIndexController().getWorkerByIndexId(NGramIndex.ID);
-        MemTreeBuilder builder = context.getDocumentBuilder();
-        DocumentBuilderReceiver docBuilder = new DocumentBuilderReceiver(builder);
-        MatchCallback matchCb = new MatchCallback(func, docBuilder);
-        Serializer serializer = context.getBroker().getSerializer();
-        serializer.reset();
-        ValueSequence result = new ValueSequence();
-        for (SequenceIterator i = args[0].iterate(); i.hasNext(); ) {
-            NodeValue v = (NodeValue) i.nextItem();
-            try {
-                int nodeNr = builder.getDocument().getLastNode();
-                if (v.getImplementationType() == NodeValue.IN_MEMORY_NODE) {
-                    ((NodeImpl)v).copyTo(context.getBroker(), docBuilder);
-                } else {
-                    NodeProxy p = (NodeProxy) v;
-                    MatchListener ml = index.getMatchListener(context.getBroker(), p, matchCb);
-                    Receiver receiver;
-                    if (ml == null)
-                        receiver = docBuilder;
-                    else {
-                        ml.setNextInChain(docBuilder);
-                        receiver = ml;
+            NGramIndexWorker index = (NGramIndexWorker) context.getBroker().getIndexController().getWorkerByIndexId(NGramIndex.ID);
+            MemTreeBuilder builder = context.getDocumentBuilder();
+            DocumentBuilderReceiver docBuilder = new DocumentBuilderReceiver(builder);
+            MatchCallback matchCb = new MatchCallback(func, docBuilder);
+            Serializer serializer = context.getBroker().getSerializer();
+            serializer.reset();
+            ValueSequence result = new ValueSequence();
+            for (SequenceIterator i = args[0].iterate(); i.hasNext(); ) {
+                NodeValue v = (NodeValue) i.nextItem();
+                try {
+                    int nodeNr = builder.getDocument().getLastNode();
+                    if (v.getImplementationType() == NodeValue.IN_MEMORY_NODE) {
+                        ((NodeImpl) v).copyTo(context.getBroker(), docBuilder);
+                    } else {
+                        NodeProxy p = (NodeProxy) v;
+                        MatchListener ml = index.getMatchListener(context.getBroker(), p, matchCb);
+                        Receiver receiver;
+                        if (ml == null)
+                            receiver = docBuilder;
+                        else {
+                            ml.setNextInChain(docBuilder);
+                            receiver = ml;
+                        }
+                        serializer.setReceiver(receiver);
+                        serializer.toReceiver((NodeProxy) v, false);
                     }
-                    serializer.setReceiver(receiver);
-                    serializer.toReceiver((NodeProxy) v, false);
+                    result.add(builder.getDocument().getNode(++nodeNr));
+                } catch (SAXException e) {
+                    LOG.warn(e.getMessage(), e);
+                    throw new XPathException(this, e.getMessage());
                 }
-                result.add(builder.getDocument().getNode(++nodeNr));
-            } catch (SAXException e) {
-                LOG.warn(e.getMessage(), e);
-                throw new XPathException(this, e.getMessage());
             }
+            return result;
         }
-        return result;
     }
 
     private class MatchCallback implements NGramMatchCallback {

extensions/indexes/range/src/org/exist/xquery/modules/range/IndexKeys.java

@@ -54,30 +54,31 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro
         if (args.length == 4) {
             start = args[arg++].getStringValue();
         }
-        final FunctionReference ref = (FunctionReference) args[arg++].itemAt(0);
-        int max = -1;
-        if (!args[arg].isEmpty()) {
-            max = ((IntegerValue)args[arg].itemAt(0)).getInt();
-        }
+        try (final FunctionReference ref = (FunctionReference) args[arg++].itemAt(0)) {
+            int max = -1;
+            if (!args[arg].isEmpty()) {
+                max = ((IntegerValue) args[arg].itemAt(0)).getInt();
+            }
 
-        final Sequence result = new ValueSequence();
-        final RangeIndexWorker worker = (RangeIndexWorker) context.getBroker().getIndexController().getWorkerByIndexName("range-index");
-        Occurrences[] occur = worker.scanIndexByField(field, contextSequence == null ? context.getStaticallyKnownDocuments() : contextSequence.getDocumentSet(), start, max);
-        final int len = (max != -1 && occur.length > max ? max : occur.length);
-        final Sequence params[] = new Sequence[2];
-        ValueSequence data = new ValueSequence();
-        for (int j = 0; j < len; j++) {
-            params[0] = new StringValue(occur[j].getTerm().toString());
-            data.add(new IntegerValue(occur[j].getOccurrences(),
-                    Type.UNSIGNED_INT));
-            data.add(new IntegerValue(occur[j].getDocuments(),
-                    Type.UNSIGNED_INT));
-            data.add(new IntegerValue(j + 1, Type.UNSIGNED_INT));
-            params[1] = data;
+            final Sequence result = new ValueSequence();
+            final RangeIndexWorker worker = (RangeIndexWorker) context.getBroker().getIndexController().getWorkerByIndexName("range-index");
+            Occurrences[] occur = worker.scanIndexByField(field, contextSequence == null ? context.getStaticallyKnownDocuments() : contextSequence.getDocumentSet(), start, max);
+            final int len = (max != -1 && occur.length > max ? max : occur.length);
+            final Sequence params[] = new Sequence[2];
+            ValueSequence data = new ValueSequence();
+            for (int j = 0; j < len; j++) {
+                params[0] = new StringValue(occur[j].getTerm().toString());
+                data.add(new IntegerValue(occur[j].getOccurrences(),
+                        Type.UNSIGNED_INT));
+                data.add(new IntegerValue(occur[j].getDocuments(),
+                        Type.UNSIGNED_INT));
+                data.add(new IntegerValue(j + 1, Type.UNSIGNED_INT));
+                params[1] = data;
 
-            result.addAll(ref.evalFunction(Sequence.EMPTY_SEQUENCE, null, params));
-            data.clear();
+                result.addAll(ref.evalFunction(Sequence.EMPTY_SEQUENCE, null, params));
+                data.clear();
+            }
+            return result;
         }
-        return result;
     }
 }

extensions/modules/src/org/exist/xquery/modules/compression/AbstractExtractFunction.java

@@ -85,7 +85,7 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) throws XPathExce
             throw new XPathException("entry-filter function must take at least 3 arguments.");
 
         filterParam = args[2];
-        
+
         //get the entry-data function and check its types
         if(!(args[3].itemAt(0) instanceof FunctionReference))
             throw new XPathException("No entry-data function provided.");
@@ -109,7 +109,10 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) throws XPathExce
             return processCompressedData(compressedData, encoding);
         } catch(final UnsupportedCharsetException | XMLDBException e) {
             throw new XPathException(this, e.getMessage(), e);
-		}
+		} finally {
+            entryDataFunction.close();
+            entryFilterFunction.close();
+        }
     }
 
     /**

extensions/modules/src/org/exist/xquery/modules/persistentlogin/PersistentLoginFunctions.java

@@ -84,7 +84,13 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro
             } else {
                 callback = null;
             }
-            return register(user, pass, timeToLive, callback);
+            try {
+                return register(user, pass, timeToLive, callback);
+            } finally {
+                if (callback != null) {
+                    callback.close();
+                }
+            }
         } else if (isCalledAs("login")) {
             final String token = args[0].getStringValue();
             final FunctionReference callback;
@@ -93,7 +99,13 @@ public Sequence eval(final Sequence[] args, final Sequence contextSequence) thro
             } else {
                 callback = null;
             }
-            return authenticate(token, callback);
+            try {
+                return authenticate(token, callback);
+            } finally {
+                if (callback != null) {
+                    callback.close();
+                }
+            }
         } else {
             PersistentLogin.getInstance().invalidate(args[0].getStringValue());
             return Sequence.EMPTY_SEQUENCE;

src/org/exist/xquery/ArrowOperator.java

@@ -109,18 +109,20 @@ public Sequence eval(Sequence contextSequence, final Item contextItem) throws XP
             }
             fref = (FunctionReference)item0;
         }
-        final List<Expression> fparams = new ArrayList<>(parameters.size() + 1);
-        fparams.add(new ContextParam(context, contextSequence));
-        fparams.addAll(parameters);
-
-        fref.setArguments(fparams);
-        // need to create a new AnalyzeContextInfo to avoid memory leak
-        // cachedContextInfo will stay in memory
-        fref.analyze(new AnalyzeContextInfo(cachedContextInfo));
-        // Evaluate the function
-        final Sequence result = fref.eval(contextSequence);
-        fref.resetState(false);
-        return result;
+        try {
+            final List<Expression> fparams = new ArrayList<>(parameters.size() + 1);
+            fparams.add(new ContextParam(context, contextSequence));
+            fparams.addAll(parameters);
+
+            fref.setArguments(fparams);
+            // need to create a new AnalyzeContextInfo to avoid memory leak
+            // cachedContextInfo will stay in memory
+            fref.analyze(new AnalyzeContextInfo(cachedContextInfo));
+            // Evaluate the function
+            return fref.eval(contextSequence);
+        } finally {
+            fref.close();
+        }
     }
 
     @Override

src/org/exist/xquery/DynamicFunctionCall.java

@@ -73,14 +73,14 @@ public Sequence eval(Sequence contextSequence, Item contextItem)
 	        ref.analyze(new AnalyzeContextInfo(cachedContextInfo));
 	        // Evaluate the function
             try {
-                final Sequence result = ref.eval(contextSequence);
-                ref.resetState(false);
-                return result;
+                return ref.eval(contextSequence);
             } catch (XPathException e) {
                 if (e.getLine() <= 0) {
                     e.setLocation(getLine(), getColumn(), getSource());
                 }
                 throw e;
+            } finally {
+                ref.close();
             }
         }
     }

src/org/exist/xquery/InlineFunction.java

@@ -96,10 +96,6 @@ public int returnsType() {
     @Override
     public void resetState(boolean postOptimization) {
         super.resetState(postOptimization);
-        // clear closure variables set on inline function
-		if (!postOptimization) {
-			calls.forEach(call -> call.getFunction().setClosureVariables(null));
-		}
         calls.clear();
         function.resetState(postOptimization);
     }

src/org/exist/xquery/UserDefinedFunction.java

@@ -278,6 +278,10 @@ public void setCaller(FunctionCall call){
 
     public void setClosureVariables(List<ClosureVariable> vars) {
     	this.closureVariables = vars;
+    	if (vars != null) {
+    		// register the closure with the context so it gets cleared after execution
+    		context.pushClosure(this);
+		}
     }
 
     public List<ClosureVariable> getClosureVariables() {

src/org/exist/xquery/XQueryContext.java

@@ -177,6 +177,9 @@ public class XQueryContext implements BinaryValueManager, Context
     // Unresolved references to user defined functions
     protected Deque<FunctionCall>                      forwardReferences                                = new ArrayDeque<>();
 
+    // Inline functions using closures need to be cleared after execution
+    protected Deque<UserDefinedFunction>               closures                                         = new ArrayDeque<>();
+
     // List of options declared for this query at compile time - i.e. declare option
     protected List<Option>                             staticOptions                                    = null;
 
@@ -1399,6 +1402,11 @@ public void reset(final boolean keepGlobals) {
         if( !isShared ) {
             lastVar = null;
         }
+
+        // clear inline functions using closures
+        closures.forEach(func -> func.setClosureVariables(null));
+        closures.clear();
+
         fragmentStack = new Stack<MemTreeBuilder>();
         callStack.clear();
         protectedDocuments = null;
@@ -2060,7 +2068,7 @@ public Map<QName, Variable> getLocalVariables() {
      */
     public List<ClosureVariable> getLocalStack() {
 
-        final List<ClosureVariable> closure = new ArrayList<>(6);
+        List<ClosureVariable> closure = null;
 
     	final LocalVariable end = contextStack.isEmpty() ? null : contextStack.peek();
 
@@ -2070,6 +2078,9 @@ public List<ClosureVariable> getLocalStack() {
                 break;
             }
 
+            if (closure == null) {
+                closure = new ArrayList<>(6);
+            }
             closure.add( new ClosureVariable(var) );
         }
 
@@ -2612,6 +2623,16 @@ public void popLocalVariables(LocalVariable var, Sequence resultSeq)
         variableStackSize--;
     }
 
+    /**
+     * Register a inline function using closure variables so it can be cleared
+     * after query execution.
+     *
+     * @param func an inline function definition using closure variables
+     */
+    public void pushClosure(final UserDefinedFunction func) {
+        closures.add(func);
+    }
+
     /**
      * Returns the current size of the stack. This is used to determine where a variable has been declared.
      *

src/org/exist/xquery/functions/array/ArrayFunction.java

@@ -1,5 +1,6 @@
 package org.exist.xquery.functions.array;
 
+import com.evolvedbinary.j8fu.function.FunctionE;
 import org.exist.dom.QName;
 import org.exist.xquery.*;
 import org.exist.xquery.value.*;
@@ -293,23 +294,24 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) throws XPathExce
                     case REVERSE:
                         return array.reverse();
                     case FOR_EACH:
-                        return array.forEach(getFunction(args[1]));
+                        return getFunction(args[1], array::forEach);
                     case FILTER:
-                        return array.filter(getFunction(args[1]));
+                        return getFunction(args[1], array::filter);
                     case FOLD_LEFT:
-                        return array.foldLeft(getFunction(args[2]), args[1]);
+                        return getFunction(args[2], ref -> array.foldLeft(ref, args[1]));
                     case FOLD_RIGHT:
-                        return array.foldRight(getFunction(args[2]), args[1]);
+                        return getFunction(args[2], ref -> array.foldRight(ref, args[1]));
                     case FOR_EACH_PAIR:
-                        return array.forEachPair((ArrayType) args[1].itemAt(0), getFunction(args[2]));
+                        return getFunction(args[2], ref -> array.forEachPair((ArrayType) args[1].itemAt(0), ref));
                 }
         }
         throw new XPathException(this, "Unknown function: " + getName());
     }
 
-    private FunctionReference getFunction(Sequence arg) throws XPathException {
-        final FunctionReference ref = (FunctionReference) arg.itemAt(0);
-        ref.analyze(cachedContextInfo);
-        return ref;
+    private Sequence getFunction(Sequence arg, FunctionE<FunctionReference, Sequence, XPathException> action) throws XPathException {
+        try (final FunctionReference ref = (FunctionReference) arg.itemAt(0)) {
+            ref.analyze(cachedContextInfo);
+            return action.apply(ref);
+        }
     }
 }