diff --git a/modules/flowable-cdi/src/main/java/org/flowable/cdi/impl/el/CdiResolver.java b/modules/flowable-cdi/src/main/java/org/flowable/cdi/impl/el/CdiResolver.java index f1d2337c758..a8982395728 100644 --- a/modules/flowable-cdi/src/main/java/org/flowable/cdi/impl/el/CdiResolver.java +++ b/modules/flowable-cdi/src/main/java/org/flowable/cdi/impl/el/CdiResolver.java @@ -41,10 +41,12 @@ */ public class CdiResolver extends ELResolver { - protected jakarta.el.ELContext context; - - public CdiResolver() { - context = new jakarta.el.ELContext() { + /** + * Creates a new context for the wrapped resolver. A {@link jakarta.el.ELContext} must not be shared between threads and the wrapped resolver + * keeps state in it (Weld keeps its stack of creational contexts there). + */ + protected jakarta.el.ELContext createELContext() { + return new jakarta.el.ELContext() { @Override public VariableMapper getVariableMapper() { @@ -63,6 +65,19 @@ public jakarta.el.ELResolver getELResolver() { }; } + /** + * Returns the context for the wrapped resolver that belongs to the given Flowable context. It is created on first use and stored on the + * Flowable context, which is created for an evaluation, so the wrapped resolver does not see the context of another evaluation. + */ + protected jakarta.el.ELContext getELContext(ELContext context) { + jakarta.el.ELContext elContext = (jakarta.el.ELContext) context.getContext(jakarta.el.ELContext.class); + if (elContext == null) { + elContext = createELContext(); + context.putContext(jakarta.el.ELContext.class, elContext); + } + return elContext; + } + protected BeanManager getBeanManager() { return BeanManagerLookup.getBeanManager(); } @@ -75,18 +90,18 @@ protected jakarta.el.ELResolver getWrappedResolver() { @Override public Class getCommonPropertyType(ELContext context, Object base) { - return getWrappedResolver().getCommonPropertyType(this.context, base); + return getWrappedResolver().getCommonPropertyType(getELContext(context), base); } @Override public Class getType(ELContext context, Object base, Object property) { - return getWrappedResolver().getType(this.context, base, property); + return getWrappedResolver().getType(getELContext(context), base, property); } @Override public Object getValue(ELContext context, Object base, Object property) { try { - Object result = getWrappedResolver().getValue(this.context, base, property); + Object result = getWrappedResolver().getValue(getELContext(context), base, property); context.setPropertyResolved(result != null); return result; } catch (IllegalStateException e) { @@ -99,17 +114,17 @@ public Object getValue(ELContext context, Object base, Object property) { @Override public boolean isReadOnly(ELContext context, Object base, Object property) { - return getWrappedResolver().isReadOnly(this.context, base, property); + return getWrappedResolver().isReadOnly(getELContext(context), base, property); } @Override public void setValue(ELContext context, Object base, Object property, Object value) { - getWrappedResolver().setValue(this.context, base, property, value); + getWrappedResolver().setValue(getELContext(context), base, property, value); } @Override public Object invoke(ELContext context, Object base, Object method, java.lang.Class[] paramTypes, Object[] params) { - Object result = getWrappedResolver().invoke(this.context, base, method, paramTypes, params); + Object result = getWrappedResolver().invoke(getELContext(context), base, method, paramTypes, params); context.setPropertyResolved(result != null); return result; } diff --git a/modules/flowable-cdi/src/test/java/org/flowable/cdi/test/impl/el/CdiResolverTest.java b/modules/flowable-cdi/src/test/java/org/flowable/cdi/test/impl/el/CdiResolverTest.java new file mode 100644 index 00000000000..871a9175038 --- /dev/null +++ b/modules/flowable-cdi/src/test/java/org/flowable/cdi/test/impl/el/CdiResolverTest.java @@ -0,0 +1,120 @@ +/* Licensed 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.flowable.cdi.test.impl.el; + +import static org.assertj.core.api.Assertions.assertThat; + +import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; +import java.util.concurrent.Future; +import java.util.concurrent.TimeUnit; + +import org.flowable.cdi.impl.el.CdiResolver; +import org.flowable.common.engine.impl.de.odysseus.el.util.SimpleContext; +import org.junit.jupiter.api.Test; + +public class CdiResolverTest { + + @Test + public void testConcurrentCallsDoNotShareTheElContext() throws Exception { + Set seenContexts = ConcurrentHashMap.newKeySet(); + CyclicBarrier bothCallsAreInside = new CyclicBarrier(2); + CdiResolver resolver = new CdiResolver() { + + @Override + protected jakarta.el.ELResolver getWrappedResolver() { + return new RecordingResolver(seenContexts, bothCallsAreInside); + } + }; + + ExecutorService executor = Executors.newFixedThreadPool(2); + try { + Future first = executor.submit(() -> resolver.getValue(new SimpleContext(), null, "bean")); + Future second = executor.submit(() -> resolver.getValue(new SimpleContext(), null, "bean")); + first.get(10, TimeUnit.SECONDS); + second.get(10, TimeUnit.SECONDS); + } finally { + executor.shutdownNow(); + } + + // The resolver of the CDI container keeps state in the context, so two calls that run at the same time must not use the same one + assertThat(seenContexts).hasSize(2); + } + + @Test + public void testElContextIsStoredOnTheFlowableContext() { + Set seenContexts = ConcurrentHashMap.newKeySet(); + CdiResolver resolver = new CdiResolver() { + + @Override + protected jakarta.el.ELResolver getWrappedResolver() { + return new RecordingResolver(seenContexts, null); + } + }; + + SimpleContext flowableContext = new SimpleContext(); + resolver.getValue(flowableContext, null, "bean"); + resolver.getValue(flowableContext, null, "bean"); + assertThat(seenContexts).hasSize(1); + + resolver.getValue(new SimpleContext(), null, "bean"); + assertThat(seenContexts).hasSize(2); + } + + protected static class RecordingResolver extends jakarta.el.ELResolver { + + protected final Set seenContexts; + protected final CyclicBarrier bothCallsAreInside; + + public RecordingResolver(Set seenContexts, CyclicBarrier bothCallsAreInside) { + this.seenContexts = seenContexts; + this.bothCallsAreInside = bothCallsAreInside; + } + + @Override + public Object getValue(jakarta.el.ELContext context, Object base, Object property) { + seenContexts.add(context); + if (bothCallsAreInside != null) { + try { + // Both calls are inside the resolver at the same time before either of them returns + bothCallsAreInside.await(10, TimeUnit.SECONDS); + } catch (Exception e) { + throw new IllegalStateException(e); + } + } + return null; + } + + @Override + public Class getType(jakarta.el.ELContext context, Object base, Object property) { + return null; + } + + @Override + public void setValue(jakarta.el.ELContext context, Object base, Object property, Object value) { + } + + @Override + public boolean isReadOnly(jakarta.el.ELContext context, Object base, Object property) { + return false; + } + + @Override + public Class getCommonPropertyType(jakarta.el.ELContext context, Object base) { + return null; + } + } +}