From a88db6771a4376a13adfe0f58967274e1c5d8882 Mon Sep 17 00:00:00 2001 From: Sebastian Baunsgaard Date: Tue, 23 Jun 2026 11:59:20 +0000 Subject: [PATCH] [MINOR] Fix class-init deadlock between AOffset and its subclasses AOffset initialized a cached empty slice in its static initializer by instantiating its own OffsetEmpty subclass. Since OffsetEmpty (and the other offset subclasses) depend on AOffset being initialized first, this formed a superclass/subclass class-initialization cycle. When two threads first touched the offset classes concurrently (e.g. parallel test execution), each could hold one class's init monitor while waiting for the other, deadlocking on the JVM class-initialization monitors. Such a deadlock is invisible to the JVM deadlock detector and cannot be interrupted, so the affected JVM hangs indefinitely. It only manifests under concurrent first-touch, which is why it never reproduced in single-threaded local runs. Defer the empty slice to a lazy holder accessed via emptySlice(), so AOffset's static initializer no longer references any subclass. By the time the holder is touched, AOffset is already initialized, so no cycle exists. Add a regression test that forces concurrent first-initialization of the offset classes through a dedicated class loader across repeated rounds and fails if it does not complete promptly. --- .../compress/colgroup/offset/AOffset.java | 25 +++- .../compress/colgroup/offset/OffsetEmpty.java | 2 +- .../colgroup/offset/OffsetSingle.java | 2 +- .../compress/colgroup/offset/OffsetTwo.java | 4 +- .../OffsetClassInitConcurrencyTest.java | 137 ++++++++++++++++++ 5 files changed, 162 insertions(+), 8 deletions(-) create mode 100644 src/test/java/org/apache/sysds/test/component/compress/offset/OffsetClassInitConcurrencyTest.java diff --git a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/AOffset.java b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/AOffset.java index 8930074eb0e..a961c1188bf 100644 --- a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/AOffset.java +++ b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/AOffset.java @@ -55,8 +55,25 @@ public abstract class AOffset implements Serializable { protected static final Log LOG = LogFactory.getLog(AOffset.class.getName()); - /** Cached final empty slice to return in cases of empty slice returns to avoid object allocation */ - protected static final OffsetSliceInfo EMPTY_SLICE = new OffsetSliceInfo(-1, -1, new OffsetEmpty()); + /** + * Lazy holder for the cached empty slice. The empty slice is built on first use rather than in AOffset's + * static initializer: instantiating the OffsetEmpty subclass from AOffset's {@code } forms a + * superclass/subclass class-initialization cycle that deadlocks when several threads first touch the offset + * classes concurrently (e.g. parallel tests). Deferring it to first use guarantees AOffset is already + * initialized by the time OffsetEmpty is loaded, so no cycle exists. + */ + private static final class EmptySliceHolder { + static final OffsetSliceInfo EMPTY_SLICE = new OffsetSliceInfo(-1, -1, new OffsetEmpty()); + } + + /** + * Get the cached empty slice, returned for empty slice results to avoid object allocation. + * + * @return the shared empty {@link OffsetSliceInfo} + */ + protected static OffsetSliceInfo emptySlice() { + return EmptySliceHolder.EMPTY_SLICE; + } /** The skip list stride size, aka how many indexes skipped for each index. */ protected static final int SKIP_STRIDE = 1000; @@ -570,12 +587,12 @@ public OffsetSliceInfo slice(int l, int u) { return new OffsetSliceInfo(0, s, moveIndex(l)); } else if (u < first) - return EMPTY_SLICE; + return emptySlice(); final AIterator it = getIteratorSkipCache(l); if(it == null || it.value() >= u) - return EMPTY_SLICE; + return emptySlice(); if(u >= last) // If including the last do not iterate. return constructSliceReturn(l, u, it.getDataIndex(), s - 1, it.getOffsetsIndex(), getLength(), it.value(), diff --git a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetEmpty.java b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetEmpty.java index 73264c84767..acd3b0d04eb 100644 --- a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetEmpty.java +++ b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetEmpty.java @@ -93,7 +93,7 @@ public int getSize() { @Override public OffsetSliceInfo slice(int l, int u) { - return EMPTY_SLICE; + return emptySlice(); } @Override diff --git a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetSingle.java b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetSingle.java index 98dab591bf9..66b9010371a 100644 --- a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetSingle.java +++ b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetSingle.java @@ -93,7 +93,7 @@ public OffsetSliceInfo slice(int l, int u) { if(l <= off && u > off) return new OffsetSliceInfo(0, 1, new OffsetSingle(off - l)); else - return EMPTY_SLICE; + return emptySlice(); } @Override diff --git a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetTwo.java b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetTwo.java index 48ce65f171f..d18c66188e4 100644 --- a/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetTwo.java +++ b/src/main/java/org/apache/sysds/runtime/compress/colgroup/offset/OffsetTwo.java @@ -98,7 +98,7 @@ public static OffsetTwo readFields(DataInput in) throws IOException { public OffsetSliceInfo slice(int l, int u) { if(l <= first) { if(u <= first) - return EMPTY_SLICE; + return emptySlice(); else if(u > last) return new OffsetSliceInfo(0, 2, moveIndex(l)); else @@ -107,7 +107,7 @@ else if(u > last) else if(l <= last && u > last) return new OffsetSliceInfo(1, 2, new OffsetSingle(last - l)); else - return EMPTY_SLICE; + return emptySlice(); } @Override diff --git a/src/test/java/org/apache/sysds/test/component/compress/offset/OffsetClassInitConcurrencyTest.java b/src/test/java/org/apache/sysds/test/component/compress/offset/OffsetClassInitConcurrencyTest.java new file mode 100644 index 00000000000..8907c82f1d1 --- /dev/null +++ b/src/test/java/org/apache/sysds/test/component/compress/offset/OffsetClassInitConcurrencyTest.java @@ -0,0 +1,137 @@ +/* + * 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.sysds.test.component.compress.offset; + +import static org.junit.Assert.fail; + +import java.io.IOException; +import java.io.InputStream; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.atomic.AtomicReference; + +import org.junit.Test; + +/** + * Regression guard for a superclass/subclass class-initialization deadlock in the offset hierarchy. + *

+ * {@code AOffset} previously instantiated its {@code OffsetEmpty} subclass from a {@code static final} field, so + * {@code AOffset.} depended on {@code OffsetEmpty} while {@code OffsetEmpty} (being a subclass) depends on + * {@code AOffset}. Initializing the two classes from different threads at the same time deadlocked on the JVM class + * initialization monitors -- which only happens under concurrent first-touch (e.g. parallel tests) and is invisible to + * the JVM deadlock detector, so it hangs forever. + *

+ * This test forces a fresh, concurrent first-initialization of the offset classes through a dedicated class loader and + * fails if it does not complete promptly. + */ +public class OffsetClassInitConcurrencyTest { + + private static final String PKG = "org.apache.sysds.runtime.compress.colgroup.offset."; + + /** Classes whose static initializers participate in the (former) cycle. */ + private static final String[] INIT_TARGETS = {PKG + "AOffset", PKG + "OffsetEmpty", PKG + "OffsetChar", + PKG + "OffsetByte", PKG + "OffsetSingle", PKG + "OffsetTwo"}; + + /** Whether a class-init cycle deadlocks depends on thread timing, so repeat to make a regression reliable to catch. */ + private static final int ROUNDS = 20; + + /** A real init deadlock never resolves; a healthy round finishes in milliseconds, so this bound is generous. */ + private static final long ROUND_TIMEOUT_MS = 10000; + + @Test(timeout = 60000) + public void concurrentFirstInitDoesNotDeadlock() throws Exception { + for(int round = 0; round < ROUNDS; round++) + runConcurrentInitRound(round); + } + + private static void runConcurrentInitRound(int round) throws Exception { + // A fresh loader per round so the offset classes initialize from scratch (rather than reusing state from + // an earlier round or earlier test), reproducing the concurrent first-touch race. + final ClassLoader loader = new OffsetPackageClassLoader(OffsetClassInitConcurrencyTest.class.getClassLoader()); + final CyclicBarrier startLine = new CyclicBarrier(INIT_TARGETS.length); + final List threads = new ArrayList<>(); + final AtomicReference failure = new AtomicReference<>(); + + for(String target : INIT_TARGETS) { + final Thread t = new Thread(() -> { + try { + startLine.await(); + // init=true forces the static initializer to run on this thread. + Class.forName(target, true, loader); + } + catch(Throwable e) { + failure.compareAndSet(null, e); + } + }, "init-" + target.substring(PKG.length())); + // Daemon so a regression (deadlock) cannot keep the JVM alive after the test times out. + t.setDaemon(true); + threads.add(t); + t.start(); + } + + final long deadline = System.currentTimeMillis() + ROUND_TIMEOUT_MS; + for(Thread t : threads) { + final long remaining = deadline - System.currentTimeMillis(); + if(remaining > 0) + t.join(remaining); + if(t.isAlive()) + fail("Concurrent class initialization deadlocked in round " + round + " (thread " + t.getName() + + " did not finish); likely a static-init cycle between AOffset and a subclass."); + } + + if(failure.get() != null) + fail("Concurrent class initialization failed in round " + round + ": " + failure.get()); + } + + /** Loads the offset package classes itself (delegating everything else) so they initialize fresh. */ + private static final class OffsetPackageClassLoader extends ClassLoader { + OffsetPackageClassLoader(ClassLoader parent) { + super(parent); + } + + @Override + protected Class loadClass(String name, boolean resolve) throws ClassNotFoundException { + if(!name.startsWith(PKG)) + return super.loadClass(name, resolve); + synchronized(getClassLoadingLock(name)) { + Class c = findLoadedClass(name); + if(c == null) + c = defineFromParentResource(name); + if(resolve) + resolveClass(c); + return c; + } + } + + private Class defineFromParentResource(String name) throws ClassNotFoundException { + final String path = name.replace('.', '/') + ".class"; + try(InputStream is = getParent().getResourceAsStream(path)) { + if(is == null) + throw new ClassNotFoundException(name); + final byte[] b = is.readAllBytes(); + return defineClass(name, b, 0, b.length); + } + catch(IOException e) { + throw new ClassNotFoundException(name, e); + } + } + } +}