[GR-49300] Break the initialization cycle in NIO/cgroups code.

PullRequest: graal/15726

Author: christianhaeubl

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java

@@ -24,14 +24,15 @@
  */
 package com.oracle.svm.core.heap;
 
+import java.util.concurrent.locks.ReentrantLock;
+
 import org.graalvm.nativeimage.ImageSingletons;
 import org.graalvm.word.UnsignedWord;
 import org.graalvm.word.WordFactory;
 
 import com.oracle.svm.core.Containers;
 import com.oracle.svm.core.SubstrateOptions;
 import com.oracle.svm.core.Uninterruptible;
-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;
@@ -55,7 +56,7 @@ default boolean hasSize() {
         UnsignedWord size();
     }
 
-    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;
 
@@ -64,6 +65,10 @@ public static boolean isInitialized() {
         return cachedSize != UNSET_SENTINEL;
     }
 
+    public static boolean isInitializationInProgress() {
+        return LOCK.isHeldByCurrentThread();
+    }
+
     /**
      * Returns the size of physical memory in bytes, querying it from the OS if it has not been
      * initialized yet.
@@ -81,23 +86,21 @@ public static UnsignedWord size() {
         }
 
         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 {
-                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();
+            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();
                 }
-            } finally {
-                INITIALIZING.decrementAndGet();
             }
         }
 

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(long value) {
+        /* Nothing to do. */
+    }
+
+    // Checkstyle: resume
+}

substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java

@@ -35,6 +35,7 @@
 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 jdk.internal.misc.Unsafe;
@@ -67,36 +68,28 @@ public static ClassLoader latestUserDefinedLoader0() {
 
     @Alias @InjectAccessors(DirectMemoryAccessors.class) //
     private static long directMemory;
-    @Alias @InjectAccessors(DirectMemoryAccessors.class) //
+    @Alias @InjectAccessors(PageAlignDirectMemoryAccessors.class) //
     private static boolean pageAlignDirectMemory;
 }
 
 final class DirectMemoryAccessors {
+    private static final long DIRECT_MEMORY_DURING_INITIALIZATION = 25 * 1024 * 1024;
 
     /*
      * 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 long directMemory;
-    private static boolean pageAlignDirectMemory;
 
     static long getDirectMemory() {
         if (!initialized) {
-            initialize();
+            return tryInitialize();
         }
         return directMemory;
     }
 
-    static boolean getPageAlignDirectMemory() {
-        if (!initialized) {
-            initialize();
-        }
-        return pageAlignDirectMemory;
-    }
-
-    private static void initialize() {
+    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
@@ -109,6 +102,15 @@ private static void initialize() {
              * No value explicitly specified. The default in the JDK in this case is the maximum
              * heap size.
              */
+            if (PhysicalMemory.isInitializationInProgress()) {
+                /*
+                 * 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.
+                 */
+                return DIRECT_MEMORY_DURING_INITIALIZATION;
+            }
             newDirectMemory = Runtime.getRuntime().maxMemory();
         }
 
@@ -118,6 +120,31 @@ private static void initialize() {
          * possible but not a case we care about.
          */
         directMemory = newDirectMemory;
+
+        /* 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() {
+        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. */