Skip to content

Commit 5fc5553

Browse files
dizzzzdizzzz
authored
Merge pull request #4636 from evolvedbinary/hotfix/xquery-context-and-update-listener-leaks
Fix Memory leaks caused by XQuery context and update listener leaks
2 parents 5d2a128 + 3fbe410 commit 5fc5553

File tree

19 files changed

+388
-262
lines changed

19 files changed

+388
-262
lines changed

‎exist-core/src/main/java/org/exist/collections/triggers/XQueryTrigger.java‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,8 @@ private void prepare(final TriggerEvent event, final DBBroker broker, final Txn
257257
} catch (final XPathException | PermissionDeniedException e) {
258258
TriggerStatePerThread.clear();
259259
throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e);
260+
} finally {
261+
context.runCleanupTasks();
260262
}
261263
}
262264

@@ -299,6 +301,8 @@ private void finish(final TriggerEvent event, final DBBroker broker, final Txn t
299301
} catch (final PermissionDeniedException e) {
300302
//Should never be reached
301303
LOG.error(e);
304+
} finally {
305+
context.runCleanupTasks();
302306
}
303307

304308
TriggerStatePerThread.clearIfFinished(TriggerPhase.AFTER);
@@ -455,6 +459,7 @@ private void execute(final TriggerPhase phase, final TriggerEvent event, final D
455459
throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e);
456460
} finally {
457461
compiledQuery.reset();
462+
context.runCleanupTasks();
458463
}
459464

460465
TriggerStatePerThread.clearIfFinished(phase);

‎exist-core/src/main/java/org/exist/dom/persistent/SortedNodeSet.java‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -288,6 +288,9 @@ public IteratorItem(final NodeProxy proxy, final PathExpr expr) {
288288
value = buf.toString();
289289
} catch(final XPathException e) {
290290
LOG.warn(e.getMessage(), e); //TODO : throw exception ! -pb
291+
} finally {
292+
expr.getContext().runCleanupTasks();
293+
expr.getContext().reset();
291294
}
292295
}
293296

‎exist-core/src/main/java/org/exist/repo/Deployment.java‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -697,6 +697,8 @@ private Sequence runQuery(final DBBroker broker, final XmldbURI targetCollection
697697
return xqs.execute(broker, compiled, null);
698698
} catch (final PermissionDeniedException e) {
699699
throw new PackageException(e.getMessage(), e);
700+
} finally {
701+
ctx.runCleanupTasks();
700702
}
701703
}
702704

‎exist-core/src/main/java/org/exist/security/internal/SMEvents.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,7 @@ protected void runScript(Subject subject, String scriptURI, String script, QName
142142
pm.queryCompleted(context.getWatchDog());
143143
}
144144
compiled.reset();
145+
context.runCleanupTasks();
145146
context.reset();
146147
}
147148

‎exist-core/src/main/java/org/exist/storage/NotificationService.java‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,8 @@ public synchronized void unsubscribe(final UpdateListener listener) {
7777
* Notify all subscribers that a document has been updated/removed or
7878
* a new document has been added.
7979
*
80-
* @param document subscribers are listining to
81-
* @param event that triggers the notify
80+
* @param document subscribers are listening to
81+
* @param event the event that triggers the notification
8282
*/
8383
public synchronized void notifyUpdate(final DocumentImpl document, final int event) {
8484
listeners.keySet().forEach(listener -> listener.documentUpdated(document, event));
@@ -100,4 +100,12 @@ public synchronized void debug() {
100100
}
101101
listeners.keySet().forEach(UpdateListener::debug);
102102
}
103+
104+
@Override
105+
public void shutdown() {
106+
synchronized (this) {
107+
LOG.warn("Expected 0 listeners at shutdown, but {} listeners are still registered", listeners.size());
108+
}
109+
BrokerPoolService.super.shutdown();
110+
}
103111
}

‎exist-core/src/main/java/org/exist/test/runner/AbstractTestRunner.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ protected static Sequence executeQuery(final BrokerPool brokerPool, final Source
7373
final XQueryPool queryPool = brokerPool.getXQueryPool();
7474
CompiledXQuery compiledQuery = queryPool.borrowCompiledXQuery(broker, query);
7575

76+
XQueryContext context = null;
7677
try {
77-
XQueryContext context;
7878
if (compiledQuery == null) {
7979
context = new XQueryContext(broker.getBrokerPool());
8080
} else {
@@ -112,6 +112,10 @@ protected static Sequence executeQuery(final BrokerPool brokerPool, final Source
112112
return xqueryService.execute(broker, compiledQuery, null);
113113

114114
} finally {
115+
if (context != null) {
116+
context.runCleanupTasks();
117+
}
118+
115119
queryPool.returnCompiledXQuery(query, compiledQuery);
116120
}
117121
}

‎exist-core/src/main/java/org/exist/test/runner/XQueryTestRunner.java‎

Lines changed: 54 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -100,65 +100,70 @@ private static XQueryTestInfo extractTestInfo(final Path path) throws Initializa
100100
}
101101

102102
final XQueryContext xqueryContext = new XQueryContext(config);
103-
xqueryContext.setTestRepository(Optional.of(expathRepo));
104-
105-
final Source xquerySource = new FileSource(path, UTF_8, false);
106-
final XQuery xquery = new XQuery();
107-
108-
final CompiledXQuery compiledXQuery = xquery.compile(xqueryContext, xquerySource);
109-
110-
String moduleNsPrefix = null;
111-
String moduleNsUri = null;
112-
final List<XQueryTestInfo.TestFunctionDef> testFunctions = new ArrayList<>();
113-
114-
final Iterator<UserDefinedFunction> localFunctions = compiledXQuery.getContext().localFunctions();
115-
while (localFunctions.hasNext()) {
116-
final UserDefinedFunction localFunction = localFunctions.next();
117-
final FunctionSignature localFunctionSignature = localFunction.getSignature();
118-
119-
String testName = null;
120-
boolean isTest = false;
121-
122-
final Annotation[] annotations = localFunctionSignature.getAnnotations();
123-
if (annotations != null) {
124-
for (final Annotation annotation : annotations) {
125-
final QName annotationName = annotation.getName();
126-
if (annotationName.getNamespaceURI().equals(XQSUITE_NAMESPACE)) {
127-
if (annotationName.getLocalPart().startsWith("assert")) {
128-
isTest = true;
129-
if (testName != null) {
130-
break;
131-
}
132-
} else if (annotationName.getLocalPart().equals("name")) {
133-
final LiteralValue[] annotationValues = annotation.getValue();
134-
if (annotationValues != null && annotationValues.length > 0) {
135-
testName = annotationValues[0].getValue().getStringValue();
136-
if (isTest) {
103+
try {
104+
xqueryContext.setTestRepository(Optional.of(expathRepo));
105+
106+
final Source xquerySource = new FileSource(path, UTF_8, false);
107+
final XQuery xquery = new XQuery();
108+
109+
final CompiledXQuery compiledXQuery = xquery.compile(xqueryContext, xquerySource);
110+
111+
String moduleNsPrefix = null;
112+
String moduleNsUri = null;
113+
final List<XQueryTestInfo.TestFunctionDef> testFunctions = new ArrayList<>();
114+
115+
final Iterator<UserDefinedFunction> localFunctions = compiledXQuery.getContext().localFunctions();
116+
while (localFunctions.hasNext()) {
117+
final UserDefinedFunction localFunction = localFunctions.next();
118+
final FunctionSignature localFunctionSignature = localFunction.getSignature();
119+
120+
String testName = null;
121+
boolean isTest = false;
122+
123+
final Annotation[] annotations = localFunctionSignature.getAnnotations();
124+
if (annotations != null) {
125+
for (final Annotation annotation : annotations) {
126+
final QName annotationName = annotation.getName();
127+
if (annotationName.getNamespaceURI().equals(XQSUITE_NAMESPACE)) {
128+
if (annotationName.getLocalPart().startsWith("assert")) {
129+
isTest = true;
130+
if (testName != null) {
137131
break;
138132
}
133+
} else if (annotationName.getLocalPart().equals("name")) {
134+
final LiteralValue[] annotationValues = annotation.getValue();
135+
if (annotationValues != null && annotationValues.length > 0) {
136+
testName = annotationValues[0].getValue().getStringValue();
137+
if (isTest) {
138+
break;
139+
}
140+
}
139141
}
140142
}
141143
}
142144
}
143-
}
144145

145-
if (isTest) {
146-
if (testName == null) {
147-
testName = localFunctionSignature.getName().getLocalPart();
148-
}
146+
if (isTest) {
147+
if (testName == null) {
148+
testName = localFunctionSignature.getName().getLocalPart();
149+
}
149150

150-
if (moduleNsPrefix == null) {
151-
moduleNsPrefix = localFunctionSignature.getName().getPrefix();
152-
}
153-
if (moduleNsUri == null) {
154-
moduleNsUri = localFunctionSignature.getName().getNamespaceURI();
155-
}
151+
if (moduleNsPrefix == null) {
152+
moduleNsPrefix = localFunctionSignature.getName().getPrefix();
153+
}
154+
if (moduleNsUri == null) {
155+
moduleNsUri = localFunctionSignature.getName().getNamespaceURI();
156+
}
156157

157-
testFunctions.add(new XQueryTestInfo.TestFunctionDef(testName));
158-
}
159-
} // end while
158+
testFunctions.add(new XQueryTestInfo.TestFunctionDef(testName));
159+
}
160+
} // end while
160161

161-
return new XQueryTestInfo(moduleNsPrefix, moduleNsUri, testFunctions);
162+
return new XQueryTestInfo(moduleNsPrefix, moduleNsUri, testFunctions);
163+
} finally {
164+
xqueryContext.runCleanupTasks();
165+
xqueryContext.reset();
166+
}
162167

163168
} catch (final DatabaseConfigurationException | IOException | PermissionDeniedException | XPathException e) {
164169
throw new InitializationError(e);

‎exist-core/src/main/java/org/exist/validation/internal/DatabaseResources.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,10 +132,10 @@ public Sequence executeQuery(String queryPath, Map<String,String> params, Subjec
132132
}
133133

134134
Sequence result= null;
135+
final XQueryContext context = new XQueryContext(brokerPool);
135136
try(final DBBroker broker = brokerPool.get(Optional.ofNullable(user))) {
136137

137138
final XQuery xquery = brokerPool.getXQueryService();
138-
final XQueryContext context = new XQueryContext(brokerPool);
139139

140140
if(collection!=null){
141141
context.declareVariable(COLLECTION, collection);
@@ -160,6 +160,7 @@ public Sequence executeQuery(String queryPath, Map<String,String> params, Subjec
160160
} catch (final EXistException | XPathException | IOException | PermissionDeniedException ex) {
161161
logger.error("Problem executing xquery", ex);
162162
result= null;
163+
context.runCleanupTasks();
163164

164165
}
165166
return result;

‎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);

0 commit comments

Comments
 (0)