This is an automated email from the ASF dual-hosted git repository.
mihaibudiu pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/calcite.git
The following commit(s) were added to refs/heads/main by this push:
new fb7b7fa748 [CALCITE-7872] Concurrent class initialization of
RelCollations and RelCollationImpl should not deadlock
fb7b7fa748 is described below
commit fb7b7fa74822b4f6835803eab9d0a5d304860a98
Author: Alan Jin <[email protected]>
AuthorDate: Wed Oct 21 07:06:46 2026 +0000
[CALCITE-7872] Concurrent class initialization of RelCollations and
RelCollationImpl should not deadlock
---
.../org/apache/calcite/rel/RelCollationImpl.java | 25 ++++-
.../java/org/apache/calcite/rel/RelCollations.java | 9 +-
.../org/apache/calcite/rel/RelCollationTest.java | 115 +++++++++++++++++++++
3 files changed, 138 insertions(+), 11 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/rel/RelCollationImpl.java
b/core/src/main/java/org/apache/calcite/rel/RelCollationImpl.java
index 19219f741a..45d231a36e 100644
--- a/core/src/main/java/org/apache/calcite/rel/RelCollationImpl.java
+++ b/core/src/main/java/org/apache/calcite/rel/RelCollationImpl.java
@@ -42,11 +42,26 @@
public class RelCollationImpl implements RelCollation {
//~ Static fields/initializers ---------------------------------------------
+ // The static initializer of RelCollations creates instances of this class,
+ // so the static initializer of this class, including the constructor that
+ // it calls, must not use RelCollations; otherwise two threads that
+ // initialize the two classes at the same time can deadlock.
+ // RelCollations.EMPTY is canonized too, so it is the same object as EMPTY.
+
@Deprecated // to be removed before 2.0
- public static final RelCollation EMPTY = RelCollations.EMPTY;
+ public static final RelCollation EMPTY =
+ RelCollationTraitDef.INSTANCE.canonize(
+ new RelCollationImpl(ImmutableList.of()));
@Deprecated // to be removed before 2.0
- public static final RelCollation PRESERVE = RelCollations.PRESERVE;
+ public static final RelCollation PRESERVE =
+ RelCollationTraitDef.INSTANCE.canonize(
+ new RelCollationImpl(
+ ImmutableList.of(new RelFieldCollation(-1))) {
+ @Override public String toString() {
+ return "PRESERVE";
+ }
+ });
//~ Instance fields --------------------------------------------------------
@@ -56,7 +71,11 @@ public class RelCollationImpl implements RelCollation {
protected RelCollationImpl(ImmutableList<RelFieldCollation> fieldCollations)
{
this.fieldCollations = fieldCollations;
- checkArgument(Util.isDistinct(RelCollations.ordinals(fieldCollations)),
+ // Not RelCollations.ordinals: this constructor runs in the static
+ // initializer of this class, which must not use RelCollations (see EMPTY).
+ checkArgument(
+ Util.isDistinct(
+ Util.transform(fieldCollations, RelFieldCollation::getFieldIndex)),
"fields must be distinct");
}
diff --git a/core/src/main/java/org/apache/calcite/rel/RelCollations.java
b/core/src/main/java/org/apache/calcite/rel/RelCollations.java
index e5107e4206..16f4cddf51 100644
--- a/core/src/main/java/org/apache/calcite/rel/RelCollations.java
+++ b/core/src/main/java/org/apache/calcite/rel/RelCollations.java
@@ -51,14 +51,7 @@ public class RelCollations {
* implementation choice is to apply operations that preserve order.
*/
@Deprecated // to be removed before 2.0
- public static final RelCollation PRESERVE =
- RelCollationTraitDef.INSTANCE.canonize(
- new RelCollationImpl(
- ImmutableList.of(new RelFieldCollation(-1))) {
- @Override public String toString() {
- return "PRESERVE";
- }
- });
+ public static final RelCollation PRESERVE = RelCollationImpl.PRESERVE;
private RelCollations() {}
diff --git a/core/src/test/java/org/apache/calcite/rel/RelCollationTest.java
b/core/src/test/java/org/apache/calcite/rel/RelCollationTest.java
index 58bf0b6374..9d4b51d60a 100644
--- a/core/src/test/java/org/apache/calcite/rel/RelCollationTest.java
+++ b/core/src/test/java/org/apache/calcite/rel/RelCollationTest.java
@@ -21,12 +21,18 @@
import org.apache.calcite.util.mapping.Mappings;
import com.google.common.collect.Lists;
+import com.google.common.io.ByteStreams;
import org.junit.jupiter.api.Test;
+import java.io.IOException;
+import java.io.InputStream;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
+import java.util.concurrent.CyclicBarrier;
+import java.util.concurrent.FutureTask;
+import java.util.concurrent.TimeUnit;
import static org.apache.calcite.rel.RelCollations.EMPTY;
import static org.apache.calcite.rel.RelFieldCollation.Direction.ASCENDING;
@@ -37,6 +43,8 @@
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.notNullValue;
+import static org.hamcrest.CoreMatchers.sameInstance;
import static org.hamcrest.MatcherAssert.assertThat;
/**
@@ -220,6 +228,71 @@ class RelCollationTest {
assertThat(CLUSTERED.reverse(), is(CLUSTERED));
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7872";>[CALCITE-7872]
+ * RelCollations and RelCollationImpl can deadlock during class
+ * initialization</a>.
+ *
+ * <p>Tests that the deprecated constants {@code RelCollationImpl.EMPTY} and
+ * {@code RelCollationImpl.PRESERVE} are the same objects as
+ * {@link RelCollations#EMPTY} and {@link RelCollations#PRESERVE}, whichever
+ * of the two classes is initialized first. They used to be null if
+ * {@code RelCollations} was initialized first. */
+ @Test void testDeprecatedConstantsInEitherInitializationOrder()
+ throws Exception {
+ final List<Class<?>> classes =
+ Arrays.asList(RelCollations.class, RelCollationImpl.class);
+ for (List<Class<?>> order : Arrays.asList(classes,
Lists.reverse(classes))) {
+ final ClassLoader loader = new IsolatingClassLoader();
+ for (Class<?> c : order) {
+ Class.forName(c.getName(), true, loader);
+ }
+ final Class<?> relCollations =
+ loader.loadClass(RelCollations.class.getName());
+ final Class<?> relCollationImpl =
+ loader.loadClass(RelCollationImpl.class.getName());
+ for (String name : Arrays.asList("EMPTY", "PRESERVE")) {
+ final Object expected = relCollations.getField(name).get(null);
+ assertThat(expected, notNullValue());
+ assertThat(relCollationImpl.getField(name).get(null),
+ sameInstance(expected));
+ }
+ }
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7872";>[CALCITE-7872]
+ * RelCollations and RelCollationImpl can deadlock during class
+ * initialization</a>.
+ *
+ * <p>Tests that two threads can initialize {@link RelCollations} and
+ * {@link RelCollationImpl} at the same time. The static initializers of the
+ * two classes used to depend on each other, so the threads deadlocked. */
+ @Test void testConcurrentClassInitialization() throws Exception {
+ for (int i = 0; i < 5; i++) {
+ final ClassLoader loader = new IsolatingClassLoader();
+ final CyclicBarrier barrier = new CyclicBarrier(2);
+ final List<FutureTask<Class<?>>> tasks = new ArrayList<>();
+ for (Class<?> c
+ : Arrays.asList(RelCollations.class, RelCollationImpl.class)) {
+ final FutureTask<Class<?>> task =
+ new FutureTask<>(() -> {
+ barrier.await();
+ return Class.forName(c.getName(), true, loader);
+ });
+ final Thread thread = new Thread(task);
+ // A thread stuck in a static initializer cannot be stopped
+ thread.setDaemon(true);
+ thread.start();
+ tasks.add(task);
+ }
+ for (FutureTask<Class<?>> task : tasks) {
+ // Throws TimeoutException if the threads deadlock
+ assertThat(task.get(10, TimeUnit.SECONDS), notNullValue());
+ }
+ }
+ }
+
private static RelCollation collation(int... ordinals) {
final List<RelFieldCollation> list = new ArrayList<>();
for (int ordinal : ordinals) {
@@ -231,4 +304,46 @@ private static RelCollation collation(int... ordinals) {
private static Mapping mapping(int sourceCount, int... sources) {
return Mappings.target(ImmutableIntList.of(sources), sourceCount);
}
+
+ /** Class loader that defines its own copy of each Calcite class, so that the
+ * static initializers of those classes run again, and delegates other
+ * classes to its parent. */
+ private static class IsolatingClassLoader extends ClassLoader {
+ private final ClassLoader parent;
+
+ IsolatingClassLoader() {
+ this(RelCollationTest.class.getClassLoader());
+ }
+
+ private IsolatingClassLoader(ClassLoader parent) {
+ super(parent);
+ this.parent = parent;
+ }
+
+ @Override protected Class<?> loadClass(String name, boolean resolve)
+ throws ClassNotFoundException {
+ if (!name.startsWith("org.apache.calcite.")) {
+ return super.loadClass(name, resolve);
+ }
+ synchronized (getClassLoadingLock(name)) {
+ Class<?> c = findLoadedClass(name);
+ if (c == null) {
+ final String resource = name.replace('.', '/') + ".class";
+ try (InputStream in = parent.getResourceAsStream(resource)) {
+ if (in == null) {
+ throw new ClassNotFoundException(name);
+ }
+ final byte[] bytes = ByteStreams.toByteArray(in);
+ c = defineClass(name, bytes, 0, bytes.length);
+ } catch (IOException e) {
+ throw new ClassNotFoundException(name, e);
+ }
+ }
+ if (resolve) {
+ resolveClass(c);
+ }
+ return c;
+ }
+ }
+ }
}
