Skip to content

Commit

Permalink
Break the initialization cycle in NIO/cgroups code
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@jerboaa
jerboaa committed Sep 14, 2023
1 parent 588ae3a commit 8329f13
Show file tree
Hide file tree
Showing 2 changed files with 128 additions and 39 deletions.
66 changes: 48 additions & 18 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/heap/PhysicalMemory.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@
*/
package com.oracle.svm.core.heap;

import java.util.concurrent.CountDownLatch;
import java.util.concurrent.TimeUnit;

import org.graalvm.nativeimage.ImageSingletons;
import org.graalvm.word.UnsignedWord;
import org.graalvm.word.WordFactory;
Expand Down Expand Up @@ -54,12 +57,21 @@ 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 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;
}

/**
Expand All @@ -78,24 +90,42 @@ 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 {
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();
synchronized (INITIALIZING) {
if (!isInitialized()) {
VMError.guarantee(INITIALIZING.get() <= 1, "Must not initialize twice");
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();
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();
}
// Now that we have set the cachedSize let other threads know it's
// available to use.
CACHED_SIZE_AVAIL_LATCH.countDown();
} finally {
INITIALIZING.incrementAndGet();
}
} finally {
INITIALIZING.decrementAndGet();
}
}

Expand Down
101 changes: 80 additions & 21 deletions ...tevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand Down Expand Up @@ -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 //
@SuppressWarnings("unused")
public static void initLevel(int newVal) {
throw VMError.shouldNotReachHere("This is an alias to the method in the target class, so this code is unreachable");
}
}

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 volatile 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();
private static synchronized void initialize() {
if (INIT_COUNT.get() == 2) {
/*
* Shouldn't really happen, but safeguard for recursive init anyway
*/
return;
}
return pageAlignDirectMemory;
}

private static void initialize() {
/*
* The JDK method VM.saveAndRemoveProperties looks at the system property
* "sun.nio.MaxDirectMemorySize". However, that property is always set by the Java HotSpot
Expand All @@ -107,17 +118,65 @@ 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.
*/
newDirectMemory = Runtime.getRuntime().maxMemory();
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, "Second cycle needs to have init count 1");
/*
* Once we know PhysicalMemory has been properly initialized we can use
* Runtime.maxMemory()
*/
INIT_COUNT.incrementAndGet();
newDirectMemory = Runtime.getRuntime().maxMemory();
}
}
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;
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. */
Expand Down

0 comments on commit 8329f13

Please sign in to comment.