This is an automated email from the ASF dual-hosted git repository. davsclaus pushed a commit to branch fix/CAMEL-24107 in repository https://gitbox.apache.org/repos/asf/camel.git
commit 3c8c413aed5cab3ba823d487e0f20a38d6ea05a2 Author: Claus Ibsen <[email protected]> AuthorDate: Thu Jul 16 11:48:20 2026 +0200 CAMEL-24107: camel-xslt - stylesheet reload is not thread-safe, stale pooled transformers keep serving the old stylesheet Co-Authored-By: Claude Opus 4.6 <[email protected]> Signed-off-by: Claus Ibsen <[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>
