This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/camel.git
The following commit(s) were added to refs/heads/main by this push:
new c51061f899fb CAMEL-24107: camel-xslt - stylesheet reload is not
thread-safe, stale pooled transformers keep serving the old stylesheet (#24776)
c51061f899fb is described below
commit c51061f899fb671c9fac01ed7ec4dcf316f3e918
Author: Claus Ibsen <[email protected]>
AuthorDate: Thu Jul 16 15:38:49 2026 +0200
CAMEL-24107: camel-xslt - stylesheet reload is not thread-safe, stale
pooled transformers keep serving the old stylesheet (#24776)
Co-Authored-By: Claude Opus 4.6 <[email protected]>
---
.../apache/camel/component/xslt/XsltBuilder.java | 67 +++++----
.../apache/camel/component/xslt/XsltEndpoint.java | 12 +-
.../camel/builder/xml/XsltBuilderReloadTest.java | 161 +++++++++++++++++++++
.../org/apache/camel/builder/xml/example2.xsl | 28 ++++
4 files changed, 239 insertions(+), 29 deletions(-)
diff --git
a/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltBuilder.java
b/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltBuilder.java
index 30fc027fde20..3874c86e8df0 100644
---
a/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltBuilder.java
+++
b/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltBuilder.java
@@ -25,6 +25,7 @@ import java.util.Map;
import java.util.Set;
import java.util.concurrent.ArrayBlockingQueue;
import java.util.concurrent.BlockingQueue;
+import java.util.concurrent.atomic.AtomicLong;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
@@ -66,7 +67,8 @@ public class XsltBuilder implements Processor {
protected static final Logger LOG =
LoggerFactory.getLogger(XsltBuilder.class);
private Map<String, Object> parameters = new HashMap<>();
- private Templates template;
+ private volatile Templates template;
+ private final AtomicLong templateGeneration = new AtomicLong();
private volatile BlockingQueue<Transformer> transformers;
private volatile SourceHandlerFactory sourceHandlerFactory;
private ResultHandlerFactory resultHandlerFactory = new
StringResultHandlerFactory();
@@ -80,6 +82,7 @@ public class XsltBuilder implements Processor {
private final XMLConverterHelper converter = new XMLConverterHelper();
private final Lock sourceHandlerFactoryLock = new ReentrantLock();
+ private final Lock transformerSourceLock = new ReentrantLock();
public XsltBuilder() {
}
@@ -103,6 +106,7 @@ public class XsltBuilder implements Processor {
exchange.getExchangeExtension().addOnCompletion(new
XsltBuilderOnCompletion(fileName));
}
+ long gen = templateGeneration.get();
Transformer transformer = getTransformer();
configureTransformer(transformer, exchange);
@@ -128,7 +132,7 @@ public class XsltBuilder implements Processor {
LOG.trace("Transform complete with result {}", result);
resultHandler.setBody(out);
} finally {
- releaseTransformer(transformer);
+ releaseTransformer(transformer, gen);
// IOHelper can handle if null
IOHelper.close(is);
}
@@ -283,6 +287,7 @@ public class XsltBuilder implements Processor {
public void setTemplate(Templates template) {
this.template = template;
+ templateGeneration.incrementAndGet();
if (transformers != null) {
transformers.clear();
}
@@ -344,28 +349,33 @@ public class XsltBuilder implements Processor {
* @throws TransformerConfigurationException is thrown if creating a XSLT
transformer failed.
*/
public void setTransformerSource(Source source) throws
TransformerConfigurationException {
- TransformerFactory factory = converter.getTransformerFactory();
- if (errorListener != null) {
- factory.setErrorListener(errorListener);
- } else {
- // use a logger error listener so users can see from the logs what
the error may be
- factory.setErrorListener(new XsltErrorListener());
- }
- if (getUriResolver() != null) {
- factory.setURIResolver(getUriResolver());
- }
+ transformerSourceLock.lock();
+ try {
+ TransformerFactory factory = converter.getTransformerFactory();
+ if (errorListener != null) {
+ factory.setErrorListener(errorListener);
+ } else {
+ // use a logger error listener so users can see from the logs
what the error may be
+ factory.setErrorListener(new XsltErrorListener());
+ }
+ if (getUriResolver() != null) {
+ factory.setURIResolver(getUriResolver());
+ }
- // Check that the call to createTemplates() returns a valid template
instance.
- // In case of a xslt parse error, it will return null, and we should
stop the
- // deployment and raise an exception as the route will not be setup
properly.
- Templates templates = createTemplates(factory, source);
- if (templates != null) {
- setTemplate(templates);
- } else {
- throw new TransformerConfigurationException(
- "Error creating XSLT template. "
- + "This is most likely
be caused by a XML parse error. "
- + "Please verify your
XSLT file configured.");
+ // Check that the call to createTemplates() returns a valid
template instance.
+ // In case of a xslt parse error, it will return null, and we
should stop the
+ // deployment and raise an exception as the route will not be
setup properly.
+ Templates templates = createTemplates(factory, source);
+ if (templates != null) {
+ setTemplate(templates);
+ } else {
+ throw new TransformerConfigurationException(
+ "Error creating XSLT template. "
+ + "This is most
likely be caused by a XML parse error. "
+ + "Please verify
your XSLT file configured.");
+ }
+ } finally {
+ transformerSourceLock.unlock();
}
}
@@ -434,13 +444,14 @@ public class XsltBuilder implements Processor {
this.xsltMessageLogger = xsltMessageLogger;
}
- private void releaseTransformer(Transformer transformer) {
+ private void releaseTransformer(Transformer transformer, long generation) {
if (transformers != null) {
- transformer.reset();
- boolean result = transformers.offer(transformer);
- if (!result) {
- LOG.error("failed to offer() a transform");
+ if (generation != templateGeneration.get()) {
+ // template was reloaded while this transformer was in use —
discard it
+ return;
}
+ transformer.reset();
+ transformers.offer(transformer);
}
}
diff --git
a/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltEndpoint.java
b/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltEndpoint.java
index f7f6b4f825d4..9e6500882af1 100644
---
a/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltEndpoint.java
+++
b/components/camel-xslt/src/main/java/org/apache/camel/component/xslt/XsltEndpoint.java
@@ -20,6 +20,8 @@ import java.io.IOException;
import java.io.InputStream;
import java.util.HashMap;
import java.util.Map;
+import java.util.concurrent.locks.Lock;
+import java.util.concurrent.locks.ReentrantLock;
import javax.xml.transform.ErrorListener;
import javax.xml.transform.Source;
@@ -63,6 +65,7 @@ public class XsltEndpoint extends ProcessorEndpoint {
private volatile boolean cacheCleared;
private volatile XsltBuilder xslt;
+ private final Lock reloadLock = new ReentrantLock();
private Map<String, Object> parameters;
@UriPath
@@ -147,7 +150,14 @@ public class XsltEndpoint extends ProcessorEndpoint {
}
}
if (!contentCache || cacheCleared) {
- loadResource(resourceUri, xslt);
+ reloadLock.lock();
+ try {
+ if (!contentCache || cacheCleared) {
+ loadResource(resourceUri, xslt);
+ }
+ } finally {
+ reloadLock.unlock();
+ }
}
super.onExchange(exchange);
}
diff --git
a/core/camel-core/src/test/java/org/apache/camel/builder/xml/XsltBuilderReloadTest.java
b/core/camel-core/src/test/java/org/apache/camel/builder/xml/XsltBuilderReloadTest.java
new file mode 100644
index 000000000000..7df571b763db
--- /dev/null
+++
b/core/camel-core/src/test/java/org/apache/camel/builder/xml/XsltBuilderReloadTest.java
@@ -0,0 +1,161 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.builder.xml;
+
+import java.net.URL;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import javax.xml.transform.stream.StreamSource;
+
+import org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.component.xslt.XsltBuilder;
+import org.apache.camel.support.DefaultExchange;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class XsltBuilderReloadTest extends ContextTestSupport {
+
+ private static final String INPUT = "<hello>world!</hello>";
+ private static final String EXPECTED_V1 = "<?xml version=\"1.0\"
encoding=\"UTF-8\"?><goodbye>world!</goodbye>";
+ private static final String EXPECTED_V2 = "<?xml version=\"1.0\"
encoding=\"UTF-8\"?><farewell>world!</farewell>";
+
+ @Test
+ public void testPooledTransformersInvalidatedOnTemplateChange() throws
Exception {
+ URL styleSheet1 = getClass().getResource("example.xsl");
+ URL styleSheet2 = getClass().getResource("example2.xsl");
+
+ XsltBuilder builder = XsltBuilder.xslt(styleSheet1);
+ builder.transformerCacheSize(10);
+
+ // process first message — transformer compiled from stylesheet 1 is
returned to pool
+ Exchange exchange1 = new DefaultExchange(context);
+ exchange1.getIn().setBody(INPUT);
+ builder.process(exchange1);
+ assertEquals(EXPECTED_V1,
exchange1.getMessage().getBody(String.class));
+
+ // reload stylesheet — pool must be invalidated
+ builder.setTransformerSource(new
StreamSource(styleSheet2.openStream()));
+
+ // process second message — must use new template, not a stale pooled
transformer
+ Exchange exchange2 = new DefaultExchange(context);
+ exchange2.getIn().setBody(INPUT);
+ builder.process(exchange2);
+ assertEquals(EXPECTED_V2,
exchange2.getMessage().getBody(String.class));
+ }
+
+ @Test
+ public void testMultipleReloadsWithTransformerCache() throws Exception {
+ URL styleSheet1 = getClass().getResource("example.xsl");
+ URL styleSheet2 = getClass().getResource("example2.xsl");
+
+ XsltBuilder builder = XsltBuilder.xslt(styleSheet1);
+ builder.transformerCacheSize(5);
+
+ // fill the pool with a few transformers from stylesheet 1
+ for (int i = 0; i < 3; i++) {
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getIn().setBody(INPUT);
+ builder.process(exchange);
+ assertEquals(EXPECTED_V1,
exchange.getMessage().getBody(String.class));
+ }
+
+ // reload to stylesheet 2
+ builder.setTransformerSource(new
StreamSource(styleSheet2.openStream()));
+
+ // all subsequent messages must use the new stylesheet
+ for (int i = 0; i < 5; i++) {
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getIn().setBody(INPUT);
+ builder.process(exchange);
+ assertEquals(EXPECTED_V2,
exchange.getMessage().getBody(String.class),
+ "Message " + i + " should use the new stylesheet");
+ }
+
+ // reload back to stylesheet 1
+ builder.setTransformerSource(new
StreamSource(styleSheet1.openStream()));
+
+ for (int i = 0; i < 3; i++) {
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getIn().setBody(INPUT);
+ builder.process(exchange);
+ assertEquals(EXPECTED_V1,
exchange.getMessage().getBody(String.class));
+ }
+ }
+
+ @Test
+ public void testConcurrentProcessingDuringReload() throws Exception {
+ URL styleSheet1 = getClass().getResource("example.xsl");
+ URL styleSheet2 = getClass().getResource("example2.xsl");
+
+ XsltBuilder builder = XsltBuilder.xslt(styleSheet1);
+ builder.transformerCacheSize(10);
+
+ // warm up the pool
+ Exchange warmup = new DefaultExchange(context);
+ warmup.getIn().setBody(INPUT);
+ builder.process(warmup);
+
+ // reload to stylesheet 2
+ builder.setTransformerSource(new
StreamSource(styleSheet2.openStream()));
+
+ // launch multiple threads that all process concurrently after the
reload
+ int threadCount = 8;
+ CountDownLatch startLatch = new CountDownLatch(1);
+ CountDownLatch doneLatch = new CountDownLatch(threadCount);
+ List<String> results = new ArrayList<>();
+ List<Throwable> errors = new ArrayList<>();
+
+ for (int i = 0; i < threadCount; i++) {
+ Thread t = new Thread(() -> {
+ try {
+ startLatch.await(10, TimeUnit.SECONDS);
+ Exchange exchange = new DefaultExchange(context);
+ exchange.getIn().setBody(INPUT);
+ builder.process(exchange);
+ synchronized (results) {
+
results.add(exchange.getMessage().getBody(String.class));
+ }
+ } catch (Throwable e) {
+ synchronized (errors) {
+ errors.add(e);
+ }
+ } finally {
+ doneLatch.countDown();
+ }
+ });
+ t.start();
+ }
+
+ // release all threads at once
+ startLatch.countDown();
+ assertTrue(doneLatch.await(30, TimeUnit.SECONDS), "All threads should
complete");
+ assertTrue(errors.isEmpty(), "No errors expected: " + errors);
+ assertEquals(threadCount, results.size());
+
+ // all results must use the new stylesheet
+ for (int i = 0; i < results.size(); i++) {
+ assertEquals(EXPECTED_V2, results.get(i),
+ "Thread " + i + " should use the new stylesheet");
+ }
+ }
+}
diff --git
a/core/camel-core/src/test/resources/org/apache/camel/builder/xml/example2.xsl
b/core/camel-core/src/test/resources/org/apache/camel/builder/xml/example2.xsl
new file mode 100644
index 000000000000..b51cd91daea4
--- /dev/null
+++
b/core/camel-core/src/test/resources/org/apache/camel/builder/xml/example2.xsl
@@ -0,0 +1,28 @@
+<?xml version = "1.0"?>
+<!--
+
+ Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements. See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License. You may obtain a copy of the License at
+
+ http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License.
+
+-->
+<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform";>
+
+ <xsl:template match="/">
+ <farewell>
+ <xsl:value-of select="/hello"/>
+ </farewell>
+ </xsl:template>
+
+</xsl:stylesheet>
