Skip to content

Commit 19a4592

Browse files
committed
[bugfix] Any additional XQueryContext that is created for the purpose of importing/compiling modules must be correctly registered with the main XQueryContext as an importedContext so that its resources are cleaned up when the query finishes. This addresses a memory leak in eXist-db that can accumulate in the NotificationService's UpdateListeners
1 parent 70d07bf commit 19a4592

File tree

7 files changed

+225
-154
lines changed

7 files changed

+225
-154
lines changed

exist-core/src/main/java/org/exist/xquery/FunctionCall.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ public Sequence evalFunction(Sequence contextSequence, Item contextItem, Sequenc
329329
@Override
330330
public void resetState(boolean postOptimization) {
331331
super.resetState(postOptimization);
332-
if(expression.needsReset() || postOptimization) {
332+
if(expression != null && (expression.needsReset() || postOptimization)) {
333333
expression.resetState(postOptimization);
334334
}
335335
}

exist-core/src/main/java/org/exist/xquery/ModuleContext.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -484,6 +484,11 @@ public void popInScopeNamespaces() {
484484
parentContext.popInScopeNamespaces();
485485
}
486486

487+
@Override
488+
public void addImportedContext(final XQueryContext importedContext) {
489+
parentContext.addImportedContext(importedContext);
490+
}
491+
487492
@Override
488493
public void registerUpdateListener(final UpdateListener listener) {
489494
parentContext.registerUpdateListener(listener);

exist-core/src/main/java/org/exist/xquery/XQueryContext.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
import java.util.concurrent.CopyOnWriteArrayList;
3535
import java.util.concurrent.atomic.AtomicReference;
3636
import java.util.function.BiFunction;
37+
import java.util.function.Consumer;
3738
import java.util.function.Predicate;
3839

3940
import javax.annotation.Nullable;
@@ -428,6 +429,26 @@ public class XQueryContext implements BinaryValueManager, Context {
428429
// Only used for testing, e.g. {@link org.exist.test.runner.XQueryTestRunner}.
429430
private Optional<ExistRepository> testRepository = Optional.empty();
430431

432+
/**
433+
* Holds a list of any new XQuery Contexts that
434+
* were created by the XQuery (owning this XQuery Context)
435+
* dynamically importing, compiling, and/or evaluating modules.
436+
*
437+
* NOTE(AR) - This is needed to ensure that these "imported contexts" are
438+
* also correctly reset and cleaned up when this XQuery is finished.
439+
*/
440+
private Set<XQueryContext> importedContexts = null;
441+
442+
/**
443+
* Holds a list of the {@link #runCleanupTasks(Predicate)} functions
444+
* of the {@link #importedContexts}.
445+
*
446+
* NOTE(AR) - This is needed to ensure that these the user call
447+
* {@link #reset()} or {@link #runCleanupTasks(Predicate)}
448+
* in any order.
449+
*/
450+
private List<Consumer<Predicate<Object>>> importedContextsCleanupTasksFns = null;
451+
431452
public XQueryContext() {
432453
this(null, null, null);
433454
}
@@ -1409,6 +1430,13 @@ public void reset() {
14091430

14101431
@Override
14111432
public void reset(final boolean keepGlobals) {
1433+
if (importedContexts != null) {
1434+
for (final XQueryContext importedContext : importedContexts) {
1435+
importedContext.reset(keepGlobals);
1436+
}
1437+
importedContexts = null;
1438+
}
1439+
14121440
setRealUser(null);
14131441

14141442
if (this.pushedUserFromHttpSession) {
@@ -2847,6 +2875,25 @@ public Map<String, Sequence> getCachedUriCollectionResults() {
28472875
return cachedUriCollectionResults;
28482876
}
28492877

2878+
/**
2879+
* Add a reference to an additional XQuery Context that
2880+
* was created by the XQuery (owning this XQuery Context)
2881+
* dynamically importing, compiling, and/or evaluating modules.
2882+
*
2883+
* NOTE(AR) - This is needed to ensure that these "imported contexts" are
2884+
* also correctly reset and cleaned up when this XQuery is finished.
2885+
*
2886+
* @param importedContext the dynamically created content for importing a module.
2887+
*/
2888+
public void addImportedContext(final XQueryContext importedContext) {
2889+
if (importedContexts == null) {
2890+
importedContexts = new HashSet<>();
2891+
importedContextsCleanupTasksFns = new ArrayList<>();
2892+
}
2893+
importedContexts.add(importedContext);
2894+
importedContextsCleanupTasksFns.add(importedContext::runCleanupTasks);
2895+
}
2896+
28502897
/**
28512898
* Save state
28522899
*/
@@ -3448,6 +3495,13 @@ public interface CleanupTask {
34483495

34493496
@Override
34503497
public void runCleanupTasks(final Predicate<Object> predicate) {
3498+
if (importedContextsCleanupTasksFns != null) {
3499+
for (final Consumer<Predicate<Object>> importedContextsCleanupTasksFn : importedContextsCleanupTasksFns) {
3500+
importedContextsCleanupTasksFn.accept(predicate);
3501+
}
3502+
importedContextsCleanupTasksFns = null;
3503+
}
3504+
34513505
for (final CleanupTask cleanupTask : cleanupTasks) {
34523506
try {
34533507
cleanupTask.cleanup(this, predicate);

exist-core/src/main/java/org/exist/xquery/functions/fn/LoadXQueryModule.java

Lines changed: 41 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -148,54 +148,58 @@ public Sequence eval(Sequence[] args, Sequence contextSequence) throws XPathExce
148148

149149
// create temporary context so main context is not polluted
150150
final XQueryContext tempContext = new XQueryContext(context.getBroker().getBrokerPool(), context.getProfiler());
151-
tempContext.setModuleLoadPath(context.getModuleLoadPath());
152-
setExternalVars(externalVars, tempContext::declareGlobalVariable);
153-
tempContext.prepareForExecution();
154-
155-
Module[] loadedModules = null;
156151
try {
157-
loadedModules = tempContext.importModule(targetNamespace, null, locationHints);
152+
tempContext.setModuleLoadPath(context.getModuleLoadPath());
153+
setExternalVars(externalVars, tempContext::declareGlobalVariable);
154+
tempContext.prepareForExecution();
155+
156+
Module[] loadedModules = null;
157+
try {
158+
loadedModules = tempContext.importModule(targetNamespace, null, locationHints);
159+
160+
} catch (final XPathException e) {
161+
if (e.getErrorCode() == ErrorCodes.XQST0059) {
162+
// importModule may throw exception if no location is given and module cannot be resolved
163+
throw new XPathException(this, ErrorCodes.FOQM0002, "Module with URI " + targetNamespace + " not found");
164+
}
165+
throw new XPathException(this, ErrorCodes.FOQM0003, "Error found when importing module: " + e.getMessage());
166+
}
158167

159-
} catch (final XPathException e) {
160-
if (e.getErrorCode() == ErrorCodes.XQST0059) {
161-
// importModule may throw exception if no location is given and module cannot be resolved
168+
// not found, raise error
169+
if (loadedModules == null || loadedModules.length == 0) {
162170
throw new XPathException(this, ErrorCodes.FOQM0002, "Module with URI " + targetNamespace + " not found");
163171
}
164-
throw new XPathException(this, ErrorCodes.FOQM0003, "Error found when importing module: " + e.getMessage());
165-
}
166172

167-
// not found, raise error
168-
if (loadedModules == null || loadedModules.length == 0) {
169-
throw new XPathException(this, ErrorCodes.FOQM0002, "Module with URI " + targetNamespace + " not found");
170-
}
173+
if (!xqVersion.equals(getXQueryVersion(tempContext.getXQueryVersion()))) {
174+
throw new XPathException(this, ErrorCodes.FOQM0003, "Imported module has wrong XQuery version: " +
175+
getXQueryVersion(tempContext.getXQueryVersion()));
176+
}
171177

172-
if (!xqVersion.equals(getXQueryVersion(tempContext.getXQueryVersion()))) {
173-
throw new XPathException(this, ErrorCodes.FOQM0003, "Imported module has wrong XQuery version: " +
174-
getXQueryVersion(tempContext.getXQueryVersion()));
175-
}
178+
final IMap<AtomicValue, Sequence> variables = newLinearMap(null);
179+
final IMap<AtomicValue, IMap<AtomicValue, Sequence>> functions = newLinearMap(null);
176180

177-
final IMap<AtomicValue, Sequence> variables = newLinearMap(null);
178-
final IMap<AtomicValue, IMap<AtomicValue, Sequence>> functions = newLinearMap(null);
181+
for (final Module loadedModule : loadedModules) {
182+
loadedModule.setContextItem(contextItem);
183+
setExternalVars(externalVars, loadedModule::declareVariable);
184+
if (!loadedModule.isInternalModule()) {
185+
// ensure variable declarations in the imported module are analyzed.
186+
// unlike when using a normal import statement, this is not done automatically
187+
((ExternalModule) loadedModule).analyzeGlobalVars();
188+
}
179189

180-
for (final Module loadedModule : loadedModules) {
181-
loadedModule.setContextItem(contextItem);
182-
setExternalVars(externalVars, loadedModule::declareVariable);
183-
if (!loadedModule.isInternalModule()) {
184-
// ensure variable declarations in the imported module are analyzed.
185-
// unlike when using a normal import statement, this is not done automatically
186-
((ExternalModule) loadedModule).analyzeGlobalVars();
190+
getModuleVariables(loadedModule, variables);
191+
getModuleFunctions(loadedModule, tempContext, functions);
187192
}
188193

189-
getModuleVariables(loadedModule, variables);
190-
getModuleFunctions(loadedModule, tempContext, functions);
191-
}
192-
193-
final IMap<AtomicValue, Sequence> result = Map.from(io.lacuna.bifurcan.List.of(
194-
new Maps.Entry<>(RESULT_FUNCTIONS, new MapType(this, context, functions.mapValues((k, v) -> (Sequence)new MapType(this, context, v.forked(), Type.INTEGER)).forked(), Type.QNAME)),
195-
new Maps.Entry<>(RESULT_VARIABLES, new MapType(this, context, variables.forked(), Type.QNAME))
196-
));
194+
final IMap<AtomicValue, Sequence> result = Map.from(io.lacuna.bifurcan.List.of(
195+
new Maps.Entry<>(RESULT_FUNCTIONS, new MapType(this, context, functions.mapValues((k, v) -> (Sequence) new MapType(this, context, v.forked(), Type.INTEGER)).forked(), Type.QNAME)),
196+
new Maps.Entry<>(RESULT_VARIABLES, new MapType(this, context, variables.forked(), Type.QNAME))
197+
));
197198

198-
return new MapType(this, context, result, Type.STRING);
199+
return new MapType(this, context, result, Type.STRING);
200+
} finally {
201+
context.addImportedContext(tempContext);
202+
}
199203
}
200204

201205
private void getModuleVariables(final Module module, final IMap<AtomicValue, Sequence> variables) throws XPathException {

exist-core/src/main/java/org/exist/xquery/functions/inspect/InspectModule.java

Lines changed: 65 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -69,77 +69,81 @@ public InspectModule(final XQueryContext context, final FunctionSignature signat
6969
public Sequence eval(final Sequence[] args, final Sequence contextSequence) throws XPathException {
7070

7171
final XQueryContext tempContext = new XQueryContext(context.getBroker().getBrokerPool());
72-
tempContext.setModuleLoadPath(context.getModuleLoadPath());
73-
final Module[] modules;
74-
if (isCalledAs(FN_INSPECT_MODULE_NAME)) {
75-
modules = tempContext.importModule(null, null, new AnyURIValue[] { (AnyURIValue) args[0].itemAt(0) });
76-
} else {
77-
modules = tempContext.importModule(args[0].getStringValue(), null, null);
78-
}
79-
80-
if (modules == null || modules.length == 0) {
81-
return Sequence.EMPTY_SEQUENCE;
82-
}
83-
84-
// this function only supports working with a singular module for a namespace!
85-
final Module module = modules[0];
86-
8772
try {
88-
context.pushDocumentContext();
89-
final MemTreeBuilder builder = context.getDocumentBuilder();
90-
final AttributesImpl attribs = new AttributesImpl();
91-
attribs.addAttribute("", "uri", "uri", "CDATA", module.getNamespaceURI());
92-
attribs.addAttribute("", "prefix", "prefix", "CDATA", module.getDefaultPrefix());
93-
if (module.isInternalModule()) {
94-
attribs.addAttribute("", "location", "location", "CDATA", "java:" + module.getClass().getName());
95-
} else if (isCalledAs("inspect-module")) {
96-
attribs.addAttribute("", "location", "location", "CDATA", args[0].getStringValue());
73+
tempContext.setModuleLoadPath(context.getModuleLoadPath());
74+
final Module[] modules;
75+
if (isCalledAs(FN_INSPECT_MODULE_NAME)) {
76+
modules = tempContext.importModule(null, null, new AnyURIValue[]{(AnyURIValue) args[0].itemAt(0)});
77+
} else {
78+
modules = tempContext.importModule(args[0].getStringValue(), null, null);
9779
}
98-
final int nodeNr = builder.startElement(MODULE_QNAME, attribs);
99-
if (!module.isInternalModule()) {
100-
XQDocHelper.parse((ExternalModule) module);
101-
}
102-
if (module.getDescription() != null) {
103-
builder.startElement(InspectFunctionHelper.DESCRIPTION_QNAME, null);
104-
builder.characters(module.getDescription());
105-
builder.endElement();
80+
81+
if (modules == null || modules.length == 0) {
82+
return Sequence.EMPTY_SEQUENCE;
10683
}
107-
if (!module.isInternalModule()) {
108-
final ExternalModule externalModule = (ExternalModule) module;
109-
if (externalModule.getMetadata() != null) {
110-
for (final Map.Entry<String, String> entry : externalModule.getMetadata().entrySet()) {
111-
builder.startElement(new QName(entry.getKey(), XMLConstants.NULL_NS_URI), null);
112-
builder.characters(entry.getValue());
113-
builder.endElement();
114-
}
84+
85+
// this function only supports working with a singular module for a namespace!
86+
final Module module = modules[0];
87+
88+
try {
89+
context.pushDocumentContext();
90+
final MemTreeBuilder builder = context.getDocumentBuilder();
91+
final AttributesImpl attribs = new AttributesImpl();
92+
attribs.addAttribute("", "uri", "uri", "CDATA", module.getNamespaceURI());
93+
attribs.addAttribute("", "prefix", "prefix", "CDATA", module.getDefaultPrefix());
94+
if (module.isInternalModule()) {
95+
attribs.addAttribute("", "location", "location", "CDATA", "java:" + module.getClass().getName());
96+
} else if (isCalledAs("inspect-module")) {
97+
attribs.addAttribute("", "location", "location", "CDATA", args[0].getStringValue());
11598
}
116-
// variables
117-
for (final VariableDeclaration var : externalModule.getVariableDeclarations()) {
118-
attribs.clear();
119-
attribs.addAttribute("", "name", "name", "CDATA", var.getName().toString());
120-
final SequenceType type = var.getSequenceType();
121-
if (type != null) {
122-
attribs.addAttribute("", "type", "type", "CDATA", Type.getTypeName(type.getPrimaryType()));
123-
attribs.addAttribute("", "cardinality", "cardinality", "CDATA", type.getCardinality().getHumanDescription());
124-
}
125-
builder.startElement(VARIABLE_QNAME, attribs);
99+
final int nodeNr = builder.startElement(MODULE_QNAME, attribs);
100+
if (!module.isInternalModule()) {
101+
XQDocHelper.parse((ExternalModule) module);
102+
}
103+
if (module.getDescription() != null) {
104+
builder.startElement(InspectFunctionHelper.DESCRIPTION_QNAME, null);
105+
builder.characters(module.getDescription());
126106
builder.endElement();
127107
}
128-
}
129-
// functions
130-
for (final FunctionSignature sig : module.listFunctions()) {
131-
if (!sig.isPrivate()) {
132-
UserDefinedFunction func = null;
133-
if (!module.isInternalModule()) {
134-
func = ((ExternalModule) module).getFunction(sig.getName(), sig.getArgumentCount(), null);
108+
if (!module.isInternalModule()) {
109+
final ExternalModule externalModule = (ExternalModule) module;
110+
if (externalModule.getMetadata() != null) {
111+
for (final Map.Entry<String, String> entry : externalModule.getMetadata().entrySet()) {
112+
builder.startElement(new QName(entry.getKey(), XMLConstants.NULL_NS_URI), null);
113+
builder.characters(entry.getValue());
114+
builder.endElement();
115+
}
116+
}
117+
// variables
118+
for (final VariableDeclaration var : externalModule.getVariableDeclarations()) {
119+
attribs.clear();
120+
attribs.addAttribute("", "name", "name", "CDATA", var.getName().toString());
121+
final SequenceType type = var.getSequenceType();
122+
if (type != null) {
123+
attribs.addAttribute("", "type", "type", "CDATA", Type.getTypeName(type.getPrimaryType()));
124+
attribs.addAttribute("", "cardinality", "cardinality", "CDATA", type.getCardinality().getHumanDescription());
125+
}
126+
builder.startElement(VARIABLE_QNAME, attribs);
127+
builder.endElement();
135128
}
136-
InspectFunctionHelper.generateDocs(sig, func, builder);
137129
}
130+
// functions
131+
for (final FunctionSignature sig : module.listFunctions()) {
132+
if (!sig.isPrivate()) {
133+
UserDefinedFunction func = null;
134+
if (!module.isInternalModule()) {
135+
func = ((ExternalModule) module).getFunction(sig.getName(), sig.getArgumentCount(), null);
136+
}
137+
InspectFunctionHelper.generateDocs(sig, func, builder);
138+
}
139+
}
140+
builder.endElement();
141+
return builder.getDocument().getNode(nodeNr);
142+
} finally {
143+
context.popDocumentContext();
138144
}
139-
builder.endElement();
140-
return builder.getDocument().getNode(nodeNr);
141145
} finally {
142-
context.popDocumentContext();
146+
context.addImportedContext(tempContext);
143147
}
144148
}
145149
}

0 commit comments

Comments
 (0)