From 34b583dcdbe9e36e667f3620987ea3d96611399e Mon Sep 17 00:00:00 2001 From: Severin Gehwolf Date: Wed, 30 Aug 2023 16:54:24 +0200 Subject: [PATCH 1/4] Break the initialization cycle in NIO/cgroups code First, we use a separate accessor for page-alignedness as it doesn't need the more sophisticated initialization of the directMemory field. Next, ensure PhysicalMemory initialization is serialized and when it is, set directMemory to a static value so that the container code can finish initialization without introducing a cyle. The final directMemory value based on the heap size is then published to JDK code by setting the VM init level to 1. Therefore, application code would use the non-static value as the upper bound. Closes: #556 (cherry picked from commit 1ae2dc011c6c74f25ef49e5cc5bd50d814545fff) --- .../oracle/svm/core/heap/PhysicalMemory.java | 51 ++++++-- .../core/jdk/Target_jdk_internal_misc_VM.java | 118 ++++++++++++++---- 2 files changed, 138 insertions(+), 31 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java index ccfbf728b12e..9b0ef407543e 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java @@ -24,6 +24,10 @@ */ package com.oracle.svm.core.heap; +import java.util.concurrent.CountDownLatch; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.locks.ReentrantLock; + import org.graalvm.nativeimage.ImageSingletons; import org.graalvm.word.UnsignedWord; import org.graalvm.word.WordFactory; @@ -54,12 +58,22 @@ default boolean hasSize() { UnsignedWord size(); } + private static final CountDownLatch CACHED_SIZE_AVAIL_LATCH = new CountDownLatch(1); private static final AtomicInteger INITIALIZING = new AtomicInteger(0); + private static final ReentrantLock LOCK = new ReentrantLock(); private static final UnsignedWord UNSET_SENTINEL = UnsignedUtils.MAX_VALUE; private static UnsignedWord cachedSize = UNSET_SENTINEL; public static boolean isInitialized() { - return cachedSize != UNSET_SENTINEL; + return INITIALIZING.get() > 1; + } + + /** + * + * @return {@code true} when PhycialMemory.size() is still initializing + */ + private static boolean isInitializing() { + return INITIALIZING.get() == 1; } /** @@ -78,13 +92,26 @@ public static UnsignedWord size() { throw VMError.shouldNotReachHere("Accessing the physical memory size may require allocation and synchronization"); } - if (!isInitialized()) { - /* - * Multiple threads can race to initialize the cache. This is OK because all of them - * will (most-likely) compute the same value. - */ - INITIALIZING.incrementAndGet(); - try { + LOCK.lock(); + try { + if (!isInitialized()) { + if (isInitializing()) { + /* + * Recursive initializations need to wait for the one initializing thread to + * finish so as to get correct reads of the cachedSize value. + */ + try { + boolean expired = !CACHED_SIZE_AVAIL_LATCH.await(1L, TimeUnit.SECONDS); + if (expired) { + throw new InternalError("Expired latch!"); + } + VMError.guarantee(cachedSize != UNSET_SENTINEL, "Expected chached size to be set"); + return cachedSize; + } catch (InterruptedException e) { + throw VMError.shouldNotReachHere("Interrupt on countdown latch!"); + } + } + INITIALIZING.incrementAndGet(); long memoryLimit = SubstrateOptions.MaxRAM.getValue(); if (memoryLimit > 0) { cachedSize = WordFactory.unsigned(memoryLimit); @@ -94,9 +121,13 @@ public static UnsignedWord size() { ? WordFactory.unsigned(memoryLimit) : ImageSingletons.lookup(PhysicalMemorySupport.class).size(); } - } finally { - INITIALIZING.decrementAndGet(); + // Now that we have set the cachedSize let other threads know it's + // available to use. + INITIALIZING.incrementAndGet(); + CACHED_SIZE_AVAIL_LATCH.countDown(); } + } finally { + LOCK.unlock(); } return cachedSize; diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java index a86d95a6755f..c14c745afed4 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java @@ -25,6 +25,7 @@ package com.oracle.svm.core.jdk; import java.util.Map; +import java.util.concurrent.atomic.AtomicInteger; import com.oracle.svm.core.NeverInline; import com.oracle.svm.core.SubstrateOptions; @@ -35,7 +36,9 @@ import com.oracle.svm.core.annotate.RecomputeFieldValue.Kind; import com.oracle.svm.core.annotate.Substitute; import com.oracle.svm.core.annotate.TargetClass; +import com.oracle.svm.core.heap.PhysicalMemory; import com.oracle.svm.core.snippets.KnownIntrinsics; +import com.oracle.svm.core.util.VMError; import jdk.internal.misc.Unsafe; @@ -67,36 +70,44 @@ public static ClassLoader latestUserDefinedLoader0() { @Alias @InjectAccessors(DirectMemoryAccessors.class) // private static long directMemory; - @Alias @InjectAccessors(DirectMemoryAccessors.class) // + @Alias @InjectAccessors(PageAlignDirectMemoryAccessors.class) // private static boolean pageAlignDirectMemory; + + @Alias // + public static native void initLevel(int newVal); + + @Alias // + public static native int initLevel(); } final class DirectMemoryAccessors { /* - * Not volatile to avoid a memory barrier when reading the values. Instead, an explicit barrier - * is inserted when writing the values. + * Full initialization is two-staged. First, we init directMemory to a static value (25MB) so + * that initialization of PhysicalMemory has a chance to finish. At that point isInintialized + * will be false, since we need to (potentially) set the value to the actual configured heap + * size. That can only be done once PhysicalMemory init completed. We'd introduce a cycle + * otherwise. */ - private static boolean initialized; - + private static boolean isInitialized; + private static final AtomicInteger INIT_COUNT = new AtomicInteger(); + private static final long STATIC_DIRECT_MEMORY_AMOUNT = 25 * 1024 * 1024; private static long directMemory; - private static boolean pageAlignDirectMemory; static long getDirectMemory() { - if (!initialized) { + if (!isInitialized) { initialize(); } return directMemory; } - static boolean getPageAlignDirectMemory() { - if (!initialized) { - initialize(); - } - return pageAlignDirectMemory; - } - private static void initialize() { + if (INIT_COUNT.get() == 2) { + /* + * Shouldn't really happen, but safeguard for recursive init anyway + */ + return; + } /* * The JDK method VM.saveAndRemoveProperties looks at the system property * "sun.nio.MaxDirectMemorySize". However, that property is always set by the Java HotSpot @@ -107,17 +118,82 @@ private static void initialize() { if (newDirectMemory == 0) { /* * No value explicitly specified. The default in the JDK in this case is the maximum - * heap size. + * heap size. However, we cannot rely on Runtime.maxMemory() until PhysicalMemory has + * fully initialized. Runtime.maxMemory() has a dependency on PhysicalMemory.size() + * which in turn depends on container support which might use NIO. To avoid this cycle, + * we first initialize the 'directMemory' field to an arbitrary value (25MB), and only + * use the Runtime.maxMemory() API once PhysicalMemory has fully initialized. + */ + if (!PhysicalMemory.isInitialized()) { + /* + * While initializing physical memory we might end up back here with an INIT_COUNT + * of 1, since we read the directMemory field during container support code + * execution which runs when PhysicalMemory is still initializing. + */ + VMError.guarantee(INIT_COUNT.get() <= 1, "Initial run needs to have init count 0 or 1"); + newDirectMemory = STATIC_DIRECT_MEMORY_AMOUNT; // Static value during initialization + INIT_COUNT.incrementAndGet(); + } else { + VMError.guarantee(INIT_COUNT.get() <= 1, "Runtime.maxMemory() invariant"); + /* + * Once we know PhysicalMemory has been properly initialized we can use + * Runtime.maxMemory(). Note that we might end up in this branch for code explicitly + * using the JDK cgroups code. At that point PhysicalMemory has likely been + * initialized. + */ + INIT_COUNT.incrementAndGet(); + newDirectMemory = Runtime.getRuntime().maxMemory(); + } + } else { + /* + * For explicitly set direct memory we are done */ - newDirectMemory = Runtime.getRuntime().maxMemory(); + Unsafe.getUnsafe().storeFence(); + directMemory = newDirectMemory; + isInitialized = true; + if (Target_jdk_internal_misc_VM.initLevel() < 1) { + // only the first accessor needs to set this + Target_jdk_internal_misc_VM.initLevel(1); + } + return; } + VMError.guarantee(newDirectMemory > 0, "New direct memory should be initialized"); - /* - * The initialization is not synchronized, so multiple threads can race. Usually this will - * lead to the same value, unless the runtime options are modified concurrently - which is - * possible but not a case we care about. - */ + Unsafe.getUnsafe().storeFence(); directMemory = newDirectMemory; + if (PhysicalMemory.isInitialized()) { + /* + * Complete initialization hand-shake once PhysicalMemory is properly initialized. Also + * set the VM init level to 1 so as to provoke the NIO code to re-set the internal + * MAX_MEMORY field. + */ + isInitialized = true; + if (Target_jdk_internal_misc_VM.initLevel() < 1) { + // only the first accessor needs to set this + Target_jdk_internal_misc_VM.initLevel(1); + } + } + } +} + +final class PageAlignDirectMemoryAccessors { + + /* + * Not volatile to avoid a memory barrier when reading the values. Instead, an explicit barrier + * is inserted when writing the values. + */ + private static boolean initialized; + + private static boolean pageAlignDirectMemory; + + static boolean getPageAlignDirectMemory() { + if (!initialized) { + initialize(); + } + return pageAlignDirectMemory; + } + + private static void initialize() { pageAlignDirectMemory = Boolean.getBoolean("sun.nio.PageAlignDirectMemory"); /* Ensure values are published to other threads before marking fields as initialized. */ From eefbe90272b42a43ca4181077af743a426591f3e Mon Sep 17 00:00:00 2001 From: Severin Gehwolf Date: Wed, 4 Oct 2023 18:37:11 +0200 Subject: [PATCH 2/4] Fix race with double init (cherry picked from commit 318f1083a75735a19f162f586ff2f815a771b1b1) --- .../com/oracle/svm/core/heap/PhysicalMemory.java | 2 +- .../core/jdk/Target_jdk_internal_misc_VM.java | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java index 9b0ef407543e..bbe902ff2c7a 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java @@ -105,7 +105,7 @@ public static UnsignedWord size() { if (expired) { throw new InternalError("Expired latch!"); } - VMError.guarantee(cachedSize != UNSET_SENTINEL, "Expected chached size to be set"); + VMError.guarantee(cachedSize != UNSET_SENTINEL, "Expected cached size to be set"); return cachedSize; } catch (InterruptedException e) { throw VMError.shouldNotReachHere("Interrupt on countdown latch!"); diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java index c14c745afed4..058880077c5c 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java @@ -90,6 +90,8 @@ final class DirectMemoryAccessors { * otherwise. */ private static boolean isInitialized; + private static final int INITIALIZING = 1; + private static final int INITIALIZED = 2; private static final AtomicInteger INIT_COUNT = new AtomicInteger(); private static final long STATIC_DIRECT_MEMORY_AMOUNT = 25 * 1024 * 1024; private static long directMemory; @@ -102,9 +104,9 @@ static long getDirectMemory() { } private static void initialize() { - if (INIT_COUNT.get() == 2) { + if (INIT_COUNT.get() == INITIALIZED) { /* - * Shouldn't really happen, but safeguard for recursive init anyway + * Safeguard for recursive init */ return; } @@ -130,18 +132,18 @@ private static void initialize() { * of 1, since we read the directMemory field during container support code * execution which runs when PhysicalMemory is still initializing. */ - VMError.guarantee(INIT_COUNT.get() <= 1, "Initial run needs to have init count 0 or 1"); + VMError.guarantee(INIT_COUNT.get() <= INITIALIZING, "Initial run needs to have init count 0 or 1"); newDirectMemory = STATIC_DIRECT_MEMORY_AMOUNT; // Static value during initialization - INIT_COUNT.incrementAndGet(); + INIT_COUNT.setRelease(INITIALIZING); } else { - VMError.guarantee(INIT_COUNT.get() <= 1, "Runtime.maxMemory() invariant"); + VMError.guarantee(INIT_COUNT.get() <= INITIALIZING, "Runtime.maxMemory() invariant"); /* * Once we know PhysicalMemory has been properly initialized we can use * Runtime.maxMemory(). Note that we might end up in this branch for code explicitly * using the JDK cgroups code. At that point PhysicalMemory has likely been * initialized. */ - INIT_COUNT.incrementAndGet(); + INIT_COUNT.setRelease(INITIALIZED); newDirectMemory = Runtime.getRuntime().maxMemory(); } } else { @@ -161,7 +163,7 @@ private static void initialize() { Unsafe.getUnsafe().storeFence(); directMemory = newDirectMemory; - if (PhysicalMemory.isInitialized()) { + if (PhysicalMemory.isInitialized() && INITIALIZED == INIT_COUNT.get()) { /* * Complete initialization hand-shake once PhysicalMemory is properly initialized. Also * set the VM init level to 1 so as to provoke the NIO code to re-set the internal From ba474f84faa9ab506ebe584c6367867e0c591387 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Thu, 5 Oct 2023 13:53:27 +0200 Subject: [PATCH 3/4] Break the initialization cycle in NIO/cgroups code in a simpler and more thread-safe way. (cherry picked from commit af74d9f0f6d0304d6bad7408c1bc92a6e4788c57) --- .../oracle/svm/core/heap/PhysicalMemory.java | 62 +++-------- .../core/jdk/Target_jdk_internal_misc_VM.java | 103 +++++------------- 2 files changed, 43 insertions(+), 122 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java index bbe902ff2c7a..cf800f59acb5 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java @@ -24,8 +24,6 @@ */ package com.oracle.svm.core.heap; -import java.util.concurrent.CountDownLatch; -import java.util.concurrent.TimeUnit; import java.util.concurrent.locks.ReentrantLock; import org.graalvm.nativeimage.ImageSingletons; @@ -34,7 +32,6 @@ import com.oracle.svm.core.Containers; import com.oracle.svm.core.SubstrateOptions; -import com.oracle.svm.core.jdk.UninterruptibleUtils.AtomicInteger; import com.oracle.svm.core.stack.StackOverflowCheck; import com.oracle.svm.core.thread.PlatformThreads; import com.oracle.svm.core.thread.VMOperation; @@ -58,22 +55,16 @@ default boolean hasSize() { UnsignedWord size(); } - private static final CountDownLatch CACHED_SIZE_AVAIL_LATCH = new CountDownLatch(1); - private static final AtomicInteger INITIALIZING = new AtomicInteger(0); private static final ReentrantLock LOCK = new ReentrantLock(); private static final UnsignedWord UNSET_SENTINEL = UnsignedUtils.MAX_VALUE; private static UnsignedWord cachedSize = UNSET_SENTINEL; public static boolean isInitialized() { - return INITIALIZING.get() > 1; + return cachedSize != UNSET_SENTINEL; } - /** - * - * @return {@code true} when PhycialMemory.size() is still initializing - */ - private static boolean isInitializing() { - return INITIALIZING.get() == 1; + public static boolean isInitializationInProgress() { + return LOCK.isHeldByCurrentThread(); } /** @@ -92,42 +83,23 @@ public static UnsignedWord size() { throw VMError.shouldNotReachHere("Accessing the physical memory size may require allocation and synchronization"); } - LOCK.lock(); - try { - if (!isInitialized()) { - if (isInitializing()) { - /* - * Recursive initializations need to wait for the one initializing thread to - * finish so as to get correct reads of the cachedSize value. - */ - try { - boolean expired = !CACHED_SIZE_AVAIL_LATCH.await(1L, TimeUnit.SECONDS); - if (expired) { - throw new InternalError("Expired latch!"); - } - VMError.guarantee(cachedSize != UNSET_SENTINEL, "Expected cached size to be set"); - return cachedSize; - } catch (InterruptedException e) { - throw VMError.shouldNotReachHere("Interrupt on countdown latch!"); + if (!isInitialized()) { + long memoryLimit = SubstrateOptions.MaxRAM.getValue(); + if (memoryLimit > 0) { + cachedSize = WordFactory.unsigned(memoryLimit); + } else { + LOCK.lock(); + try { + if (!isInitialized()) { + memoryLimit = Containers.memoryLimitInBytes(); + cachedSize = memoryLimit > 0 + ? WordFactory.unsigned(memoryLimit) + : ImageSingletons.lookup(PhysicalMemorySupport.class).size(); } + } finally { + LOCK.unlock(); } - INITIALIZING.incrementAndGet(); - long memoryLimit = SubstrateOptions.MaxRAM.getValue(); - if (memoryLimit > 0) { - cachedSize = WordFactory.unsigned(memoryLimit); - } else { - memoryLimit = Containers.memoryLimitInBytes(); - cachedSize = memoryLimit > 0 - ? WordFactory.unsigned(memoryLimit) - : ImageSingletons.lookup(PhysicalMemorySupport.class).size(); - } - // Now that we have set the cachedSize let other threads know it's - // available to use. - INITIALIZING.incrementAndGet(); - CACHED_SIZE_AVAIL_LATCH.countDown(); } - } finally { - LOCK.unlock(); } return cachedSize; diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java index 058880077c5c..fa2e5e45ba14 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java @@ -25,7 +25,6 @@ package com.oracle.svm.core.jdk; import java.util.Map; -import java.util.concurrent.atomic.AtomicInteger; import com.oracle.svm.core.NeverInline; import com.oracle.svm.core.SubstrateOptions; @@ -38,7 +37,6 @@ import com.oracle.svm.core.annotate.TargetClass; import com.oracle.svm.core.heap.PhysicalMemory; import com.oracle.svm.core.snippets.KnownIntrinsics; -import com.oracle.svm.core.util.VMError; import jdk.internal.misc.Unsafe; @@ -72,44 +70,26 @@ public static ClassLoader latestUserDefinedLoader0() { private static long directMemory; @Alias @InjectAccessors(PageAlignDirectMemoryAccessors.class) // private static boolean pageAlignDirectMemory; - - @Alias // - public static native void initLevel(int newVal); - - @Alias // - public static native int initLevel(); } final class DirectMemoryAccessors { + private static final long DIRECT_MEMORY_DURING_INITIALIZATION = 25 * 1024 * 1024; /* - * Full initialization is two-staged. First, we init directMemory to a static value (25MB) so - * that initialization of PhysicalMemory has a chance to finish. At that point isInintialized - * will be false, since we need to (potentially) set the value to the actual configured heap - * size. That can only be done once PhysicalMemory init completed. We'd introduce a cycle - * otherwise. + * Not volatile to avoid a memory barrier when reading the values. Instead, an explicit barrier + * is inserted when writing the values. */ - private static boolean isInitialized; - private static final int INITIALIZING = 1; - private static final int INITIALIZED = 2; - private static final AtomicInteger INIT_COUNT = new AtomicInteger(); - private static final long STATIC_DIRECT_MEMORY_AMOUNT = 25 * 1024 * 1024; + private static boolean initialized; private static long directMemory; static long getDirectMemory() { - if (!isInitialized) { - initialize(); + if (!initialized) { + return tryInitialize(); } return directMemory; } - private static void initialize() { - if (INIT_COUNT.get() == INITIALIZED) { - /* - * Safeguard for recursive init - */ - return; - } + private static long tryInitialize() { /* * The JDK method VM.saveAndRemoveProperties looks at the system property * "sun.nio.MaxDirectMemorySize". However, that property is always set by the Java HotSpot @@ -120,72 +100,41 @@ private static void initialize() { if (newDirectMemory == 0) { /* * No value explicitly specified. The default in the JDK in this case is the maximum - * heap size. However, we cannot rely on Runtime.maxMemory() until PhysicalMemory has - * fully initialized. Runtime.maxMemory() has a dependency on PhysicalMemory.size() - * which in turn depends on container support which might use NIO. To avoid this cycle, - * we first initialize the 'directMemory' field to an arbitrary value (25MB), and only - * use the Runtime.maxMemory() API once PhysicalMemory has fully initialized. + * heap size. */ - if (!PhysicalMemory.isInitialized()) { + if (PhysicalMemory.isInitializationInProgress()) { /* - * While initializing physical memory we might end up back here with an INIT_COUNT - * of 1, since we read the directMemory field during container support code - * execution which runs when PhysicalMemory is still initializing. + * When initializing PhysicalMemory, we use NIO/cgroups code that calls + * VM.getDirectMemory(). When this initialization is in progress, we need to prevent + * that Runtime.maxMemory() is called below because it would trigger a recursive + * initialization of PhysicalMemory. So, we return a temporary value. */ - VMError.guarantee(INIT_COUNT.get() <= INITIALIZING, "Initial run needs to have init count 0 or 1"); - newDirectMemory = STATIC_DIRECT_MEMORY_AMOUNT; // Static value during initialization - INIT_COUNT.setRelease(INITIALIZING); - } else { - VMError.guarantee(INIT_COUNT.get() <= INITIALIZING, "Runtime.maxMemory() invariant"); - /* - * Once we know PhysicalMemory has been properly initialized we can use - * Runtime.maxMemory(). Note that we might end up in this branch for code explicitly - * using the JDK cgroups code. At that point PhysicalMemory has likely been - * initialized. - */ - INIT_COUNT.setRelease(INITIALIZED); - newDirectMemory = Runtime.getRuntime().maxMemory(); - } - } else { - /* - * For explicitly set direct memory we are done - */ - Unsafe.getUnsafe().storeFence(); - directMemory = newDirectMemory; - isInitialized = true; - if (Target_jdk_internal_misc_VM.initLevel() < 1) { - // only the first accessor needs to set this - Target_jdk_internal_misc_VM.initLevel(1); + return DIRECT_MEMORY_DURING_INITIALIZATION; } - return; + newDirectMemory = Runtime.getRuntime().maxMemory(); } - VMError.guarantee(newDirectMemory > 0, "New direct memory should be initialized"); - Unsafe.getUnsafe().storeFence(); + /* + * The initialization is not synchronized, so multiple threads can race. Usually this will + * lead to the same value, unless the runtime options are modified concurrently - which is + * possible but not a case we care about. + */ directMemory = newDirectMemory; - if (PhysicalMemory.isInitialized() && INITIALIZED == INIT_COUNT.get()) { - /* - * Complete initialization hand-shake once PhysicalMemory is properly initialized. Also - * set the VM init level to 1 so as to provoke the NIO code to re-set the internal - * MAX_MEMORY field. - */ - isInitialized = true; - if (Target_jdk_internal_misc_VM.initLevel() < 1) { - // only the first accessor needs to set this - Target_jdk_internal_misc_VM.initLevel(1); - } - } + + /* Ensure values are published to other threads before marking fields as initialized. */ + Unsafe.getUnsafe().storeFence(); + initialized = true; + + return newDirectMemory; } } final class PageAlignDirectMemoryAccessors { - /* * Not volatile to avoid a memory barrier when reading the values. Instead, an explicit barrier * is inserted when writing the values. */ private static boolean initialized; - private static boolean pageAlignDirectMemory; static boolean getPageAlignDirectMemory() { From 1d2c7c1d2feaa9733647a7793c335f527c585335 Mon Sep 17 00:00:00 2001 From: Christian Haeubl Date: Fri, 6 Oct 2023 14:04:39 +0200 Subject: [PATCH 4/4] Prevent that an invalid direct memory size is cached in java.nio.Bits#MAX_MEMORY. (cherry picked from commit a60694004fd8e71c0af98fed8bbd25a066285d18) --- .../svm/core/jdk/Target_java_nio_Bits.java | 28 +++++++++++++++++-- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_nio_Bits.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_nio_Bits.java index 292659db7ded..f569e8a68bd2 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_nio_Bits.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_java_nio_Bits.java @@ -27,9 +27,11 @@ import java.util.concurrent.atomic.AtomicLong; import com.oracle.svm.core.annotate.Alias; +import com.oracle.svm.core.annotate.InjectAccessors; import com.oracle.svm.core.annotate.RecomputeFieldValue; import com.oracle.svm.core.annotate.RecomputeFieldValue.Kind; import com.oracle.svm.core.annotate.TargetClass; +import com.oracle.svm.core.heap.PhysicalMemory; @TargetClass(className = "java.nio.Bits") final class Target_java_nio_Bits { @@ -40,9 +42,9 @@ final class Target_java_nio_Bits { private static int PAGE_SIZE = -1; @Alias @RecomputeFieldValue(kind = Kind.FromAlias) // - private static boolean MEMORY_LIMIT_SET = false; - @Alias @RecomputeFieldValue(kind = Kind.FromAlias) // - private static long MAX_MEMORY = -1; + private static boolean MEMORY_LIMIT_SET = true; + @Alias @InjectAccessors(MaxMemoryAccessor.class) // + private static long MAX_MEMORY; @Alias @RecomputeFieldValue(kind = Kind.FromAlias) // private static AtomicLong RESERVED_MEMORY = new AtomicLong(); @@ -53,3 +55,23 @@ final class Target_java_nio_Bits { // Checkstyle: resume } + +/** + * {@code java.nio.Bits} caches the max. direct memory size in the field {@code MAX_MEMORY}. We + * disable this cache and always call {@link DirectMemoryAccessors#getDirectMemory()} instead, which + * uses our own cache. Otherwise, it could happen that {@code MAX_MEMORY} caches a temporary value + * that is used during early VM startup, before {@link PhysicalMemory} is fully initialized. + */ +final class MaxMemoryAccessor { + // Checkstyle: stop + + static long getMAX_MEMORY() { + return DirectMemoryAccessors.getDirectMemory(); + } + + static void setMAX_MEMORY(@SuppressWarnings("unused") long value) { + /* Nothing to do. */ + } + + // Checkstyle: resume +}