HHH-19429 Fix potential ConcurrentModificationException in AbstractSqmPath

hibernate-core/src/main/java/org/hibernate/query/sqm/tree/domain/AbstractSqmPath.java

@@ -6,10 +6,10 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Consumer;
 
 import org.hibernate.AssertionFailure;
@@ -33,7 +33,6 @@
 import jakarta.persistence.metamodel.PluralAttribute;
 import jakarta.persistence.metamodel.SingularAttribute;
 
-import static java.util.Collections.emptyList;
 import static org.hibernate.internal.util.NullnessUtil.castNonNull;
 
 /**
@@ -49,7 +48,7 @@ public abstract class AbstractSqmPath<T> extends AbstractSqmExpression<T> implem
 	 * E.g., given {@code p.mate.mate} the {@code SqmRoot} identified by {@code p} would
 	 * have a reusable path for the {@code p.mate} path.
 	 */
-	private Map<String, SqmPath<?>> reusablePaths;
+	private final Map<String, SqmPath<?>> reusablePaths = new ConcurrentHashMap<>( 0 );
 
 	protected AbstractSqmPath(
 			NavigablePath navigablePath,
@@ -103,32 +102,27 @@ public SqmPath<?> getLhs() {
 
 	@Override
 	public List<SqmPath<?>> getReusablePaths() {
-		return reusablePaths == null ? emptyList() : new ArrayList<>( reusablePaths.values() );
+		return new ArrayList<>( reusablePaths.values() );
 	}
 
 	@Override
 	public void visitReusablePaths(Consumer<SqmPath<?>> consumer) {
-		if ( reusablePaths != null ) {
-			reusablePaths.values().forEach( consumer );
-		}
+		reusablePaths.values().forEach( consumer );
 	}
 
 	@Override
 	public void registerReusablePath(SqmPath<?> path) {
 		assert path.getLhs() == this;
-		if ( reusablePaths == null ) {
-			reusablePaths = new HashMap<>();
-		}
 		final String relativeName = path.getNavigablePath().getLocalName();
-		final SqmPath<?> previous = reusablePaths.put( relativeName, path );
+		final SqmPath<?> previous = reusablePaths.putIfAbsent( relativeName, path );
 		if ( previous != null && previous != path ) {
 			throw new IllegalStateException( "Implicit-join path registration unexpectedly overrode previous registration - " + relativeName );
 		}
 	}
 
 	@Override
 	public SqmPath<?> getReusablePath(String name) {
-		return reusablePaths == null ? null : reusablePaths.get( name );
+		return reusablePaths.get( name );
 	}
 
 	@Override
@@ -206,18 +200,10 @@ protected <X> SqmPath<X> resolvePath(PersistentAttribute<?, X> attribute) {
 	protected <X> SqmPath<X> resolvePath(String attributeName, SqmPathSource<X> pathSource) {
 		final SqmPathSource<?> intermediatePathSource =
 				getResolvedModel().getIntermediatePathSource( pathSource );
-		if ( reusablePaths == null ) {
-			reusablePaths = new HashMap<>();
-			final SqmPath<X> path = pathSource.createSqmPath( this, intermediatePathSource );
-			reusablePaths.put( attributeName, path );
-			return path;
-		}
-		else {
-			//noinspection unchecked
-			return (SqmPath<X>)
-					reusablePaths.computeIfAbsent( attributeName,
-							name -> pathSource.createSqmPath( this, intermediatePathSource ) );
-		}
+		//noinspection unchecked
+		return (SqmPath<X>)
+				reusablePaths.computeIfAbsent( attributeName,
+						name -> pathSource.createSqmPath( this, intermediatePathSource ) );
 	}
 
 	protected <S extends T> SqmTreatedPath<T, S> getTreatedPath(ManagedDomainType<S> treatTarget) {

hibernate-core/src/test/java/org/hibernate/orm/test/sqm/ConcurrentQueryCreationTest.java

@@ -0,0 +1,195 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ * Copyright Red Hat Inc. and Hibernate Authors
+ */
+package org.hibernate.orm.test.sqm;
+
+import java.util.List;
+import java.util.concurrent.CompletionService;
+import java.util.concurrent.ExecutorCompletionService;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.TimeUnit;
+import java.util.function.Consumer;
+
+import org.hibernate.cfg.Configuration;
+import org.hibernate.cfg.Environment;
+import org.hibernate.engine.spi.SessionImplementor;
+
+import org.hibernate.testing.junit4.BaseCoreFunctionalTestCase;
+import org.hibernate.testing.orm.junit.JiraKey;
+import org.junit.Test;
+
+import jakarta.persistence.Entity;
+import jakarta.persistence.GeneratedValue;
+import jakarta.persistence.Id;
+import jakarta.persistence.Inheritance;
+import jakarta.persistence.InheritanceType;
+import jakarta.persistence.Version;
+
+import static org.junit.Assert.assertEquals;
+
+@JiraKey(value = "HHH-19429")
+public class ConcurrentQueryCreationTest extends BaseCoreFunctionalTestCase {
+
+	private static final int NUM_THREADS = 32;
+
+	@Override
+	protected void configure(Configuration configuration) {
+		super.configure( configuration );
+		configuration.setProperty( Environment.POOL_SIZE, Integer.toString( NUM_THREADS ) );
+	}
+
+	@Test
+	public void versionedUpdate() throws Exception {
+		Consumer<SessionImplementor> action = session -> {
+			SimpleEntity entity = new SimpleEntity( "jack" );
+			session.persist( entity );
+			session.createMutationQuery( "UPDATE VERSIONED SimpleEntity e SET e.name = :name WHERE e.id = :id" )
+					.setParameter( "id", entity.getId() )
+					.setParameter( "name", "new name" )
+					.executeUpdate();
+		};
+
+		runConcurrently( action );
+	}
+
+	@Test
+	public void queryWithTreat() throws Exception {
+		Consumer<SessionImplementor> action = session -> {
+			SpecificEntity specificEntity = new SpecificEntity( "some name" );
+			session.persist( specificEntity );
+			session.persist( new OtherSpecificEntity( "some name" ) );
+
+			List<Long> results = session.createQuery(
+							"""
+									SELECT COUNT(*) FROM ParentEntity e WHERE e.id = :id
+									AND (
+										TREAT(e as SpecificEntity).name = :name
+									OR
+										TREAT(e as OtherSpecificEntity).name = :name
+									)""", Long.class
+					)
+					.setParameter( "id", specificEntity.getId() )
+					.setParameter( "name", "some name" )
+					.getResultList();
+
+			assertEquals( 1, results.size() );
+			assertEquals( 1L, results.get( 0 ).longValue() );
+		};
+
+		runConcurrently( action );
+	}
+
+	private void runConcurrently(Consumer<SessionImplementor> action) throws Exception {
+		ExecutorService executor = Executors.newFixedThreadPool( NUM_THREADS );
+		try {
+			CompletionService<Void> completionService = new ExecutorCompletionService<>( executor );
+
+			for ( int round = 0; round < 100; round++ ) {
+				for ( int i = 0; i < NUM_THREADS; i++ ) {
+					completionService.submit( () -> {
+						inTransaction( action );
+						return null;
+					} );
+				}
+
+				for ( int i = 0; i < NUM_THREADS; i++ ) {
+					completionService.take().get( 1, TimeUnit.MINUTES );
+				}
+
+				rebuildSessionFactory();
+			}
+		}
+		finally {
+			executor.shutdown();
+			executor.awaitTermination( 1, TimeUnit.MINUTES );
+		}
+	}
+
+	@Override
+	protected Class<?>[] getAnnotatedClasses() {
+		return new Class<?>[] {
+				SimpleEntity.class,
+				ParentEntity.class,
+				SpecificEntity.class,
+				OtherSpecificEntity.class
+		};
+	}
+
+	@Override
+	protected boolean isCleanupTestDataRequired() {
+		return true;
+	}
+
+	@Entity(name = "SimpleEntity")
+	public static class SimpleEntity {
+		@Id
+		@GeneratedValue
+		private Integer id;
+
+		@Version
+		private Integer version;
+
+		private String name;
+
+		SimpleEntity() {
+		}
+
+		SimpleEntity(String name) {
+			this.name = name;
+		}
+
+		public Integer getId() {
+			return id;
+		}
+
+		public void setId(Integer id) {
+			this.id = id;
+		}
+
+		public String getName() {
+			return name;
+		}
+
+		public void setName(String name) {
+			this.name = name;
+		}
+	}
+
+	@Entity(name = "ParentEntity")
+	@Inheritance(strategy = InheritanceType.JOINED)
+	public static class ParentEntity {
+		@Id
+		@GeneratedValue
+		private Integer id;
+
+		public Integer getId() {
+			return id;
+		}
+	}
+
+	@Entity(name = "SpecificEntity")
+	public static class SpecificEntity extends ParentEntity {
+		private String name;
+
+		public SpecificEntity() {
+		}
+
+		public SpecificEntity(String name) {
+			this.name = name;
+		}
+	}
+
+	@Entity(name = "OtherSpecificEntity")
+	public static class OtherSpecificEntity extends ParentEntity {
+		private String name;
+
+		public OtherSpecificEntity() {
+		}
+
+		public OtherSpecificEntity(String name) {
+			this.name = name;
+		}
+	}
+}