Skip to content

Commit ee15dd0

Browse files
adamretteradamretter
committed
[bugfix] Ensure that a compiled XQuery that is reused is updated correctly, and that XQueryContext is always cleaned up
1 parent c645867 commit ee15dd0

File tree

28 files changed

+957
-510
lines changed

28 files changed

+957
-510
lines changed

‎exist-core/pom.xml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -904,10 +904,12 @@
904904
<include>src/main/java/org/exist/dom/persistent/VirtualNodeSet.java</include>
905905
<include>src/main/java/org/exist/dom/persistent/XMLUtil.java</include>
906906
<include>src/test/java/org/exist/http/AbstractHttpTest.java</include>
907+
<include>src/main/java/org/exist/http/AuditTrailSessionListener.java</include>
907908
<include>src/main/java/org/exist/http/Descriptor.java</include>
908909
<include>src/main/java/org/exist/http/RESTServer.java</include>
909910
<include>src/test/java/org/exist/http/RESTServiceTest.java</include>
910911
<include>src/main/java/org/exist/http/servlets/AbstractExistHttpServlet.java</include>
912+
<include>src/main/java/org/exist/http/servlets/RedirectorServlet.java</include>
911913
<include>src/main/java/org/exist/http/servlets/XQueryServlet.java</include>
912914
<include>src/main/java/org/exist/http/servlets/XSLTServlet.java</include>
913915
<include>src/main/java/org/exist/http/urlrewrite/ModuleCall.java</include>
@@ -940,6 +942,7 @@
940942
<include>src/main/java/org/exist/management/impl/DatabaseMXBean.java</include>
941943
<include>src/main/java/org/exist/management/impl/ExistMBean.java</include>
942944
<include>src/main/java/org/exist/management/impl/JMXAgent.java</include>
945+
<include>src/main/java/org/exist/management/impl/SanityReport.java</include>
943946
<include>src/main/java/org/exist/numbering/DLN.java</include>
944947
<include>src/main/java/org/exist/numbering/DLNFactory.java</include>
945948
<include>src/main/java/org/exist/numbering/NodeId.java</include>
@@ -1110,6 +1113,7 @@
11101113
<include>src/main/java/org/exist/xquery/DynamicCardinalityCheck.java</include>
11111114
<include>src/main/java/org/exist/xquery/DynamicTypeCheck.java</include>
11121115
<include>src/main/java/org/exist/xquery/DynamicVariable.java</include>
1116+
<include>src/test/java/org/exist/xquery/EmbeddedBinariesTest.java.java</include>
11131117
<include>src/main/java/org/exist/xquery/ErrorCodes.java</include>
11141118
<include>src/main/java/org/exist/xquery/ExternalModuleImpl.java</include>
11151119
<include>src/test/java/org/exist/xquery/ForwardReferenceTest.java</include>
@@ -1328,6 +1332,7 @@
13281332
<include>src/main/java/org/exist/xquery/value/Type.java</include>
13291333
<include>src/main/java/org/exist/xslt/EXistURIResolver.java</include>
13301334
<include>src/main/java/org/exist/xslt/XsltURIResolverHelper.java</include>
1335+
<include>src/main/java/org/exist/xupdate/Conditional.java</include>
13311336
<include>src/main/java/org/exist/xupdate/Modification.java</include>
13321337
<include>src/test/java/org/exist/xupdate/RemoveAppendTest.java</include>
13331338
<include>src/main/java/org/exist/xupdate/XUpdateProcessor.java</include>
@@ -1479,11 +1484,13 @@
14791484
<exclude>src/main/java/org/exist/dom/persistent/XMLDeclarationImpl.java</exclude>
14801485
<exclude>src/main/java/org/exist/dom/persistent/XMLUtil.java</exclude>
14811486
<exclude>src/test/java/org/exist/http/AbstractHttpTest.java</exclude>
1487+
<exclude>src/main/java/org/exist/http/AuditTrailSessionListener.java</exclude>
14821488
<exclude>src/main/java/org/exist/http/Descriptor.java</exclude>
14831489
<exclude>src/test/java/org/exist/http/RESTExternalVariableTest.java</exclude>
14841490
<exclude>src/main/java/org/exist/http/RESTServer.java</exclude>
14851491
<exclude>src/test/java/org/exist/http/RESTServiceTest.java</exclude>
14861492
<exclude>src/main/java/org/exist/http/servlets/AbstractExistHttpServlet.java</exclude>
1493+
<exclude>src/main/java/org/exist/http/servlets/RedirectorServlet.java</exclude>
14871494
<exclude>src/main/java/org/exist/http/servlets/XQueryServlet.java</exclude>
14881495
<exclude>src/main/java/org/exist/http/servlets/XSLTServlet.java</exclude>
14891496
<exclude>src/main/java/org/exist/http/urlrewrite/ModuleCall.java</exclude>
@@ -1517,6 +1524,7 @@
15171524
<exclude>src/main/java/org/exist/management/impl/DatabaseMXBean.java</exclude>
15181525
<exclude>src/main/java/org/exist/management/impl/ExistMBean.java</exclude>
15191526
<exclude>src/main/java/org/exist/management/impl/JMXAgent.java</exclude>
1527+
<exclude>src/main/java/org/exist/management/impl/SanityReport.java</exclude>
15201528
<exclude>src/main/java/org/exist/numbering/DLN.java</exclude>
15211529
<exclude>src/main/java/org/exist/numbering/DLNFactory.java</exclude>
15221530
<exclude>src/main/java/org/exist/numbering/NodeId.java</exclude>
@@ -1766,6 +1774,7 @@
17661774
<exclude>src/main/java/org/exist/xquery/DynamicCardinalityCheck.java</exclude>
17671775
<exclude>src/main/java/org/exist/xquery/DynamicTypeCheck.java</exclude>
17681776
<exclude>src/main/java/org/exist/xquery/DynamicVariable.java</exclude>
1777+
<exclude>src/test/java/org/exist/xquery/EmbeddedBinariesTest.java.java</exclude>
17691778
<exclude>src/main/java/org/exist/xquery/ErrorCodes.java</exclude>
17701779
<exclude>src/main/java/org/exist/xquery/ExternalModuleImpl.java</exclude>
17711780
<exclude>src/test/java/org/exist/xquery/ForwardReferenceTest.java</exclude>
@@ -2023,6 +2032,7 @@
20232032
<exclude>src/main/java/org/exist/xquery/value/YearMonthDurationValue.java</exclude>
20242033
<exclude>src/main/java/org/exist/xslt/EXistURIResolver.java</exclude>
20252034
<exclude>src/main/java/org/exist/xslt/XsltURIResolverHelper.java</exclude>
2035+
<exclude>src/main/java/org/exist/xupdate/Conditional.java</exclude>
20262036
<exclude>src/main/java/org/exist/xupdate/Modification.java</exclude>
20272037
<exclude>src/test/java/org/exist/xupdate/RemoveAppendTest.java</exclude>
20282038
<exclude>src/main/java/org/exist/xupdate/XUpdateProcessor.java</exclude>

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

Lines changed: 118 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,8 @@
7373
import org.exist.xquery.value.Sequence;
7474
import org.exist.xquery.value.StringValue;
7575

76+
import javax.annotation.Nullable;
77+
7678
import static com.evolvedbinary.j8fu.tuple.Tuple.Tuple;
7779

7880
/**
@@ -256,33 +258,47 @@ private void prepare(final TriggerEvent event, final DBBroker broker, final Txn
256258
LOG.warn(e.getMessage());
257259
return;
258260
}
259-
260-
final XQueryContext context = new XQueryContext(broker.getBrokerPool());
261-
CompiledXQuery compiledQuery = null;
262-
try {
263-
//compile the XQuery
264-
compiledQuery = service.compile(context, query);
265-
declareExternalVariables(context, TriggerPhase.BEFORE, event, src, dst, isCollection);
266-
267-
} catch (final XPathException | IOException | PermissionDeniedException e) {
268-
TriggerStatePerThread.clear();
269-
throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e);
270-
}
271261

272-
//execute the XQuery
262+
@Nullable CompiledXQuery compiled = null;
263+
@Nullable XQueryContext context = null;
273264
try {
274-
//TODO : should we provide another contextSet ?
275-
final NodeSet contextSet = NodeSet.EMPTY_SET;
276-
service.execute(broker, compiledQuery, contextSet);
277-
//TODO : should we have a special processing ?
278-
if (LOG.isDebugEnabled()) {
279-
LOG.debug("Trigger fired for prepare");
280-
}
281-
} catch (final XPathException | PermissionDeniedException e) {
282-
TriggerStatePerThread.clear();
283-
throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e);
265+
266+
compiled = broker.getBrokerPool().getXQueryPool().borrowCompiledXQuery(broker, query);
267+
if (compiled == null) {
268+
context = new XQueryContext(broker.getBrokerPool());
269+
} else {
270+
context = compiled.getContext();
271+
context.prepareForReuse();
272+
}
273+
274+
if (compiled == null) {
275+
compiled = service.compile(context, query);
276+
} else {
277+
compiled.getContext().updateContext(context);
278+
context.getWatchDog().reset();
279+
}
280+
281+
declareExternalVariables(context, TriggerPhase.BEFORE, event, src, dst, isCollection);
282+
283+
//execute the XQuery
284+
//TODO : should we provide another contextSet ?
285+
final NodeSet contextSet = NodeSet.EMPTY_SET;
286+
service.execute(broker, compiled, contextSet);
287+
//TODO : should we have a special processing ?
288+
if (LOG.isDebugEnabled()) {
289+
LOG.debug("Trigger fired for prepare");
290+
}
291+
292+
} catch (final XPathException | IOException | PermissionDeniedException e) {
293+
TriggerStatePerThread.clear();
294+
throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e);
284295
} finally {
285-
context.runCleanupTasks();
296+
if (context != null) {
297+
context.runCleanupTasks();
298+
}
299+
if (compiled != null) {
300+
broker.getBrokerPool().getXQueryPool().returnCompiledXQuery(query, compiled);
301+
}
286302
}
287303
}
288304

@@ -300,33 +316,42 @@ private void finish(final TriggerEvent event, final DBBroker broker, final Txn t
300316
LOG.warn(e.getMessage());
301317
return;
302318
}
303-
304-
final XQueryContext context = new XQueryContext(broker.getBrokerPool());
305-
CompiledXQuery compiledQuery = null;
319+
320+
@Nullable CompiledXQuery compiled = null;
321+
@Nullable XQueryContext context = null;
306322
try {
307-
//compile the XQuery
308-
compiledQuery = service.compile(context, query);
309-
declareExternalVariables(context, TriggerPhase.AFTER, event, src, dst, isCollection);
323+
compiled = broker.getBrokerPool().getXQueryPool().borrowCompiledXQuery(broker, query);
324+
if (compiled == null) {
325+
context = new XQueryContext(broker.getBrokerPool());
326+
} else {
327+
context = compiled.getContext();
328+
context.prepareForReuse();
329+
}
330+
331+
if (compiled == null) {
332+
compiled = service.compile(context, query);
333+
} else {
334+
compiled.getContext().updateContext(context);
335+
context.getWatchDog().reset();
336+
}
337+
338+
declareExternalVariables(context, TriggerPhase.AFTER, event, src, dst, isCollection);
339+
340+
//execute the XQuery
341+
//TODO : should we provide another contextSet ?
342+
final NodeSet contextSet = NodeSet.EMPTY_SET;
343+
service.execute(broker, compiled, contextSet);
344+
//TODO : should we have a special processing ?
310345

311346
} catch (final XPathException | IOException | PermissionDeniedException e) {
312-
//Should never be reached
313-
LOG.error(e);
314-
}
315-
316-
//execute the XQuery
317-
try {
318-
//TODO : should we provide another contextSet ?
319-
final NodeSet contextSet = NodeSet.EMPTY_SET;
320-
service.execute(broker, compiledQuery, contextSet);
321-
//TODO : should we have a special processing ?
322-
} catch (final XPathException e) {
323-
//Should never be reached
324-
LOG.error("Error during trigger finish", e);
325-
} catch (final PermissionDeniedException e) {
326-
//Should never be reached
327-
LOG.error(e);
347+
LOG.error("Error during trigger finish", e);
328348
} finally {
329-
context.runCleanupTasks();
349+
if (context != null) {
350+
context.runCleanupTasks();
351+
}
352+
if (compiled != null) {
353+
broker.getBrokerPool().getXQueryPool().returnCompiledXQuery(query, compiled);
354+
}
330355
}
331356

332357
TriggerStatePerThread.clearIfFinished(TriggerPhase.AFTER);
@@ -380,16 +405,29 @@ private CompiledXQuery getScript(final DBBroker broker, final Txn transaction) t
380405
if(query == null) {
381406
return null;
382407
}
383-
384-
final XQueryContext context = new XQueryContext(broker.getBrokerPool());
385-
if (query instanceof DBSource) {
386-
context.setModuleLoadPath(XmldbURI.EMBEDDED_SERVER_URI_PREFIX + ((DBSource)query).getDocumentPath().removeLastSegment().toString());
387-
}
388408

389-
CompiledXQuery compiledQuery;
409+
@Nullable CompiledXQuery compiled = null;
410+
@Nullable XQueryContext context = null;
390411
try {
412+
compiled = broker.getBrokerPool().getXQueryPool().borrowCompiledXQuery(broker, query);
413+
if (compiled == null) {
414+
context = new XQueryContext(broker.getBrokerPool());
415+
} else {
416+
context = compiled.getContext();
417+
context.prepareForReuse();
418+
}
419+
420+
if (query instanceof DBSource) {
421+
context.setModuleLoadPath(XmldbURI.EMBEDDED_SERVER_URI_PREFIX + ((DBSource)query).getDocumentPath().removeLastSegment().toString());
422+
}
423+
391424
//compile the XQuery
392-
compiledQuery = service.compile(context, query);
425+
if (compiled == null) {
426+
compiled = service.compile(context, query);
427+
} else {
428+
compiled.getContext().updateContext(context);
429+
context.getWatchDog().reset();
430+
}
393431

394432
//declare user defined parameters as external variables
395433
if (userDefinedVariables != null) {
@@ -400,17 +438,15 @@ private CompiledXQuery getScript(final DBBroker broker, final Txn transaction) t
400438
context.declareVariable(bindingPrefix + varName, true, new StringValue(varValue));
401439
}
402440
}
403-
404-
//reset & prepareForExecution for execution
405-
compiledQuery.reset();
406-
407-
context.getWatchDog().reset();
408441

409-
//do any preparation before execution
410-
context.prepareForExecution();
411-
412-
return compiledQuery;
442+
return compiled;
413443
} catch(final XPathException | IOException | PermissionDeniedException e) {
444+
if (context != null) {
445+
context.runCleanupTasks();
446+
}
447+
if (compiled != null) {
448+
broker.getBrokerPool().getXQueryPool().returnCompiledXQuery(query, compiled);
449+
}
414450
LOG.warn(e.getMessage(), e);
415451
throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e);
416452
}
@@ -430,23 +466,18 @@ private void execute(final TriggerPhase phase, final TriggerEvent event, final D
430466
LOG.debug("Execute: {} {}({}): {}", phase, event, src, getClass().getSimpleName());
431467
}
432468

433-
final CompiledXQuery compiledQuery;
469+
@Nullable CompiledXQuery compiled = null;
470+
@Nullable XQueryContext context = null;
434471
try {
435-
compiledQuery = getScript(broker, transaction);
436-
if (compiledQuery == null) {
472+
compiled = getScript(broker, transaction);
473+
if (compiled == null) {
437474
// NOTE: can occur if there is no such XQueryTrigger library module available in the database
438475
TriggerStatePerThread.clearIfFinished(phase);
439476
return;
440477
}
441-
} catch (final TriggerException e) {
442-
TriggerStatePerThread.clear();
443-
throw e;
444-
}
445-
446-
final XQueryContext context = compiledQuery.getContext();
478+
context = compiled.getContext();
447479

448-
//execute the XQuery
449-
try {
480+
//execute the XQuery
450481
final int nParams;
451482
if (dst != null) {
452483
nParams = 2;
@@ -467,13 +498,18 @@ private void execute(final TriggerPhase phase, final TriggerEvent event, final D
467498
args.add(new LiteralValue(context, new AnyURIValue(src)));
468499
}
469500

470-
service.execute(broker, compiledQuery, Tuple(functionName, args, Optional.empty()), null, null, true);
501+
service.execute(broker, compiled, Tuple(functionName, args, Optional.empty()), null, null, true);
502+
503+
} catch (final TriggerException e) {
504+
TriggerStatePerThread.clear();
505+
throw e;
506+
471507
} catch (final XPathException | PermissionDeniedException e) {
472508
// if the exception just indicates that there is no function in the trigger to call, then we can just log and return
473509
if (e instanceof XPathException xpe) {
474510
if (xpe.getErrorCode() == ErrorCodes.EXXQDY0005 || xpe.getErrorCode() == ErrorCodes.EXXQDY0006) {
475511
if (LOG.isDebugEnabled()) {
476-
LOG.debug("No such function '" + functionName + "' in XQueryTrigger: " + compiledQuery.getSource());
512+
LOG.debug("No such function '" + functionName + "' in XQueryTrigger: " + compiled.getSource());
477513
}
478514
TriggerStatePerThread.clearIfFinished(phase);
479515
return;
@@ -483,8 +519,12 @@ private void execute(final TriggerPhase phase, final TriggerEvent event, final D
483519
TriggerStatePerThread.clear();
484520
throw new TriggerException(PREPARE_EXCEPTION_MESSAGE, e);
485521
} finally {
486-
compiledQuery.reset();
487-
context.runCleanupTasks();
522+
if (context != null) {
523+
context.runCleanupTasks();
524+
}
525+
if (compiled != null) {
526+
broker.getBrokerPool().getXQueryPool().returnCompiledXQuery(compiled.getSource(), compiled);
527+
}
488528
}
489529

490530
TriggerStatePerThread.clearIfFinished(phase);

0 commit comments

Comments
 (0)