From 5881d7eeca63ff18cb01823311c24c687d835fdb Mon Sep 17 00:00:00 2001 From: Igor Mukhin Date: Mon, 16 Sep 2024 13:55:15 +0200 Subject: [PATCH] Fixes coalesce and case operators in multithreaded environments with different number of arguments (backport to 3.0) (#2263) TestArgumentListFunctionExpressionConcurrency.java - adds tests that reproduce the bug. ExpressionOperator.java - fixes getArgumentIndices by disabling the caching of the dynamically created argument indexes. Co-authored-by: Igor Mukhin --- .../expressions/ExpressionOperator.java | 19 ++- ...mentListFunctionExpressionConcurrency.java | 129 ++++++++++++++++++ 2 files changed, 141 insertions(+), 7 deletions(-) create mode 100644 jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/jpql/TestArgumentListFunctionExpressionConcurrency.java diff --git a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/expressions/ExpressionOperator.java b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/expressions/ExpressionOperator.java index 6ede993480c..28571e02a69 100644 --- a/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/expressions/ExpressionOperator.java +++ b/foundation/org.eclipse.persistence.core/src/main/java/org/eclipse/persistence/expressions/ExpressionOperator.java @@ -2379,15 +2379,12 @@ public void printCollection(List items, ExpressionSQLPrinter printer dbStringIndex = 1; } - if (this.argumentIndices == null) { - this.argumentIndices = new int[items.size()]; - for (int i = 0; i < this.argumentIndices.length; i++){ - this.argumentIndices[i] = i; - } - } + // Empty `this.argumentIndices` means the operator expects a list of arguments with a variable length. + // #2136: As operator's state is shared among all threads, we are not allowed to modify the field `this.argumentIndices`. + int[] indices = (this.argumentIndices != null ? this.argumentIndices : arrayIndexSequence(items.size())); String[] dbStrings = getDatabaseStrings(items.size()); - for (final int index : this.argumentIndices) { + for (final int index : indices) { Expression item = items.get(index); if ((this.selector == Ref) || ((this.selector == Deref) && (item.isObjectExpression()))) { DatabaseTable alias = item.aliasForTable(((ObjectExpression)item).getDescriptor().getTables().firstElement()); @@ -2403,6 +2400,14 @@ public void printCollection(List items, ExpressionSQLPrinter printer } } + private int[] arrayIndexSequence(int size) { + int[] result = new int[size]; + for (int i = 0; i < size; i++) { + result[i] = i; + } + return result; + } + /** * INTERNAL: Print the collection onto the SQL stream. */ diff --git a/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/jpql/TestArgumentListFunctionExpressionConcurrency.java b/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/jpql/TestArgumentListFunctionExpressionConcurrency.java new file mode 100644 index 00000000000..38878805808 --- /dev/null +++ b/jpa/eclipselink.jpa.test.jse/src/it/java/org/eclipse/persistence/jpa/test/jpql/TestArgumentListFunctionExpressionConcurrency.java @@ -0,0 +1,129 @@ +/* + * Copyright (c) 2024 Oracle and/or its affiliates. All rights reserved. + * Copyright (c) 2024 IBM Corporation. All rights reserved. + * + * This program and the accompanying materials are made available under the + * terms of the Eclipse Public License v. 2.0 which is available at + * http://www.eclipse.org/legal/epl-2.0, + * or the Eclipse Distribution License v. 1.0 which is available at + * http://www.eclipse.org/org/documents/edl-v10.php. + * + * SPDX-License-Identifier: EPL-2.0 OR BSD-3-Clause + */ +package org.eclipse.persistence.jpa.test.jpql; + + +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.atomic.AtomicReference; +import java.util.function.ObjIntConsumer; + +import org.eclipse.persistence.jpa.test.framework.DDLGen; +import org.eclipse.persistence.jpa.test.framework.Emf; +import org.eclipse.persistence.jpa.test.framework.EmfRunner; +import org.eclipse.persistence.jpa.test.jpql.model.JPQLEntity; +import org.junit.Test; +import org.junit.runner.RunWith; + +import jakarta.persistence.EntityManager; +import jakarta.persistence.EntityManagerFactory; + +/** + * This test reproduces the issues #2136, #1867 and #1717. + * + * @author Igor Mukhin + */ +@RunWith(EmfRunner.class) +public class TestArgumentListFunctionExpressionConcurrency { + + private static final int MAX_THREADS = Math.min(Runtime.getRuntime().availableProcessors(), 4); + private static final int ITERATIONS_PER_THREAD = 1000; + + @Emf(name = "argumentListFunctionExpressionConcurrencyEMF", createTables = DDLGen.DROP_CREATE, classes = { JPQLEntity.class }) + private EntityManagerFactory emf; + + @Test + public void testConcurrentUseOfCoalesce() throws Exception { + runInParallel((em, i) -> { + String jpql; + if (i % 2 == 0) { + jpql = "SELECT p FROM JPQLEntity p WHERE p.string1 = coalesce(p.string2, '" + cacheBuster(i) + "')"; + } else { + jpql = "SELECT p FROM JPQLEntity p WHERE p.string1 = coalesce(p.string2, p.string1, '" + cacheBuster(i) + "')"; + } + em.createQuery(jpql, JPQLEntity.class).getResultList(); + System.out.println(Thread.currentThread().getName() + " - " + i % 2); + }); + } + + @Test + public void testConcurrentUseOfCaseCondition() throws Exception { + runInParallel((em, i) -> { + String jpql; + if (i % 2 == 0) { + jpql = "SELECT p FROM JPQLEntity p" + + " WHERE p.string1 = case " + + " when p.string2 = '" + cacheBuster(i) + "' then p.string1 " + + " else null " + + " end"; + } else { + jpql = "SELECT p FROM JPQLEntity p" + + " WHERE p.string1 = case " + + " when p.string2 = '" + cacheBuster(i) + "' then p.string1" + + " when p.string2 = 'x' then p.string2" + + " else null " + + " end"; + + } + em.createQuery(jpql, JPQLEntity.class).getResultList(); + }); + } + + private static String cacheBuster(Integer i) { + return "cacheBuster." + Thread.currentThread().getName() + "." + i; + } + + private void runInParallel(ObjIntConsumer runnable) throws Exception { + AtomicReference exception = new AtomicReference<>(); + + // start all threads + List threads = new ArrayList<>(); + for (int t = 0; t < MAX_THREADS; t++) { + Thread thread = new Thread(() -> { + try { + for (int i = 0; i < ITERATIONS_PER_THREAD; i++) { + if (exception.get() != null) { + return; + } + + EntityManager em = emf.createEntityManager(); + try { + runnable.accept(em, i); + } finally { + em.close(); + } + + } + } catch (Exception e) { + exception.set(e); + } + }); + threads.add(thread); + thread.start(); + } + + // wait for all threads to finish + threads.forEach(thread -> { + try { + thread.join(); + } catch (InterruptedException e) { + exception.set(e); + } + }); + + // throw the first exception that occurred + if (exception.get() != null) { + throw exception.get(); + } + } +}