[bugfix] RestXQ was not releasing file handles on Binary Files access…

…ed in Resource Functions
eXist-db · Nov 12, 2018 · d36abb9 · d36abb9
1 parent 69ddcd9
commit d36abb9
Show file tree

Hide file tree

Showing 3 changed files with 38 additions and 17 deletions.
diff --git a/.../src/main/java/org/exist/extensions/exquery/restxq/impl/ResourceFunctionExecutorImpl.java b/.../src/main/java/org/exist/extensions/exquery/restxq/impl/ResourceFunctionExecutorImpl.java
@@ -50,15 +50,7 @@
 import org.exist.storage.DBBroker;
 import org.exist.storage.ProcessMonitor;
 import org.exist.xmldb.XmldbURI;
-import org.exist.xquery.AbstractExpression;
-import org.exist.xquery.AnalyzeContextInfo;
-import org.exist.xquery.CompiledXQuery;
-import org.exist.xquery.Expression;
-import org.exist.xquery.FunctionCall;
-import org.exist.xquery.UserDefinedFunction;
-import org.exist.xquery.VariableDeclaration;
-import org.exist.xquery.XPathException;
-import org.exist.xquery.XQueryContext;
+import org.exist.xquery.*;
 import org.exist.xquery.util.ExpressionDumper;
 import org.exist.xquery.value.AnyURIValue;
 import org.exist.xquery.value.AtomicValue;
@@ -175,7 +167,17 @@ public Sequence execute(final ResourceFunction resourceFunction, final Iterable<
                 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);
+
+                    // copy for closure
+                    final CompiledXQuery xquery1 = xquery;
+
+                    // return a sequence adapter which returns the query when it is finished with the results
+                    return new SequenceAdapter(result, () -> {
+                        if (xquery1 != null) {
+                            //return the compiled query to the pool
+                            cache.returnCompiledQuery(resourceFunction.getXQueryLocation(), xquery1);
+                        }
+                    });
                 } finally {
                     //switch back from effective user if setUid/setGid
                     if (effectiveSubject.isPresent()) {
@@ -185,19 +187,20 @@ public Sequence execute(final ResourceFunction resourceFunction, final Iterable<
             }
 
         } catch(final URISyntaxException | EXistException | XPathException | PermissionDeniedException use) {
+
+            // if an error occurred we should return the compiled query
+            if(xquery != null) {
+                //return the compiled query to the pool
+                cache.returnCompiledQuery(resourceFunction.getXQueryLocation(), xquery);
+            }
+
             throw new RestXqServiceException(use.getMessage(), use);
         } finally {
-
             //clear down monitoring
             if(processMonitor != null) {
                 xquery.getContext().getProfiler().traceQueryEnd(xquery.getContext());
                 processMonitor.queryCompleted(xquery.getContext().getWatchDog());
             }
-
-            if(xquery != null) {
-                //return the compiled query to the pool
-                cache.returnCompiledQuery(resourceFunction.getXQueryLocation(), xquery);
-            }
         }
     }
 

diff --git a/...n/java/org/exist/extensions/exquery/restxq/impl/RestXqServiceCompiledXQueryCacheImpl.java b/...n/java/org/exist/extensions/exquery/restxq/impl/RestXqServiceCompiledXQueryCacheImpl.java
@@ -92,6 +92,7 @@ public CompiledXQuery getCompiledQuery(final DBBroker broker, final URI xqueryLo
     @Override
     public void returnCompiledQuery(final URI xqueryLocation, final CompiledXQuery xquery) {
         //reset the query and context
+        xquery.getContext().runCleanupTasks();
         xquery.reset();
         xquery.getContext().reset();
 

diff --git a/...stxq/src/main/java/org/exist/extensions/exquery/restxq/impl/adapters/SequenceAdapter.java b/...stxq/src/main/java/org/exist/extensions/exquery/restxq/impl/adapters/SequenceAdapter.java
@@ -27,6 +27,8 @@
 package org.exist.extensions.exquery.restxq.impl.adapters;
 
 import java.util.Iterator;
+
+import com.evolvedbinary.j8fu.function.RunnableE;
 import org.apache.logging.log4j.LogManager;
 import org.apache.logging.log4j.Logger;
 import org.exist.dom.persistent.NodeProxy;
@@ -37,6 +39,8 @@
 import org.exquery.xquery.Type;
 import org.exquery.xquery.TypedValue;
 
+import javax.annotation.Nullable;
+
 /**
  *
  * @author Adam Retter <adam.retter@googlemail.com>
@@ -46,9 +50,15 @@ public class SequenceAdapter implements Sequence<Item> {
     private final static Logger LOG = LogManager.getLogger(SequenceAdapter.class);
 
     private final org.exist.xquery.value.Sequence sequence;
+    @Nullable private final RunnableE<SequenceException> closer;
 
     public SequenceAdapter(final org.exist.xquery.value.Sequence sequence) {
+        this(sequence, null);
+    }
+
+    public SequenceAdapter(final org.exist.xquery.value.Sequence sequence, @Nullable final RunnableE<SequenceException> closer) {
         this.sequence = sequence;
+        this.closer = closer;
     }
 
     @Override
@@ -123,8 +133,15 @@ public Sequence<Item> tail() {
             return new SequenceAdapter(org.exist.xquery.value.Sequence.EMPTY_SEQUENCE);
         }
     }
-
+
+    @Override
+    public void close() throws SequenceException {
+        closer.run();
+    }
+
     public org.exist.xquery.value.Sequence getExistSequence() {
         return sequence;
     }
+
+
 }