Skip to content

[GR-69306] Fix rare race conditions. #12158

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Sep 12, 2025
Merged

[GR-69306] Fix rare race conditions. #12158

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 8 additions & 16 deletions substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,6 @@
package com.oracle.svm.core.jdk;

import static jdk.graal.compiler.core.common.LibGraalSupport.LIBGRAAL_SETTING_PROPERTY_PREFIX;
import static jdk.graal.compiler.nodes.extended.MembarNode.FenceKind.STORE_STORE;

import java.util.ArrayList;
import java.util.Collections;
Expand All @@ -45,14 +44,12 @@

import com.oracle.svm.core.FutureDefaultsOptions;
import com.oracle.svm.core.SubstrateOptions;
import com.oracle.svm.core.SubstrateUtil;
import com.oracle.svm.core.VM;
import com.oracle.svm.core.c.locale.LocaleSupport;
import com.oracle.svm.core.config.ConfigurationValues;
import com.oracle.svm.core.util.VMError;

import jdk.graal.compiler.api.replacements.Fold;
import jdk.graal.compiler.nodes.extended.MembarNode;

/**
* This class maintains the system properties at run time.
Expand Down Expand Up @@ -294,8 +291,8 @@ private synchronized void initializeAllProperties() {
}

/*
* No memory barrier is needed because the loop above already emits one STORE_STORE barrier
* per initialized system property.
* No memory barrier is needed here (same reasoning as for
* LazySystemProperty.markAsInitialized()).
*/
allPropertiesInitialized = true;
}
Expand All @@ -318,13 +315,7 @@ private void ensurePropertyInitialized(String key) {
}

LazySystemProperty property = lazySystemProperties.get(key);
if (property != null) {
ensureInitialized(property);
}
}

private void ensureInitialized(LazySystemProperty property) {
if (!property.isInitialized()) {
if (property != null && !property.isInitialized()) {
initializeProperty(property);
}
}
Expand Down Expand Up @@ -385,11 +376,12 @@ public String computeValue() {
return supplier.get();
}

/**
* No memory barrier is needed here because the involved maps ({@link #initialProperties}
* and {@link #currentProperties}) already use acquire/release semantics when a value is
* added or accessed.
*/
public void markAsInitialized() {
if (!SubstrateUtil.HOSTED) {
/* Ensure that other threads see consistent values once 'initialized' is true. */
MembarNode.memoryBarrier(STORE_STORE);
}
initialized = true;
}
}
Expand Down
34 changes: 11 additions & 23 deletions ...tevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/Target_jdk_internal_misc_VM.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@
import com.oracle.svm.core.annotate.TargetClass;
import com.oracle.svm.core.snippets.KnownIntrinsics;

import jdk.internal.misc.Unsafe;

@TargetClass(className = "jdk.internal.misc.VM")
public final class Target_jdk_internal_misc_VM {
/** Ensure that we do not leak the full set of properties from the image generator. */
Expand Down Expand Up @@ -72,11 +70,14 @@ public static ClassLoader latestUserDefinedLoader0() {
}

final class DirectMemoryAccessors {
/*
* Not volatile to avoid a memory barrier when reading the values. Instead, an explicit barrier
* is inserted when writing the values.
/**
* This field needs to be volatile to ensure that reads emit a LOAD-LOAD barrier. Without this
* barrier, subsequent reads could be reordered before the read of {@link #initialized},
* allowing threads to observe an uninitialized value for {@link #directMemory}. We could
* directly emit a LOAD-LOAD barrier instead, but it doesn't make any difference in terms of the
* used instructions on any of the relevant CPU architectures.
*/
private static boolean initialized;
private static volatile boolean initialized;
private static long directMemory;

static long getDirectMemory() {
Expand All @@ -102,27 +103,16 @@ private static long tryInitialize() {
newDirectMemory = Runtime.getRuntime().maxMemory();
}

/*
* 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;

/* Ensure values are published to other threads before marking fields as initialized. */
Unsafe.getUnsafe().storeFence();
/* STORE_STORE barrier is executed as part of the volatile write. */
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;
/** See {@link DirectMemoryAccessors#initialized} on why this needs to be volatile. */
private static volatile boolean initialized;
private static boolean pageAlignDirectMemory;

static Boolean getPageAlignDirectMemory() {
Expand All @@ -134,9 +124,7 @@ static Boolean getPageAlignDirectMemory() {

private static void initialize() {
pageAlignDirectMemory = Boolean.getBoolean("sun.nio.PageAlignDirectMemory");

/* Ensure values are published to other threads before marking fields as initialized. */
Unsafe.getUnsafe().storeFence();
/* STORE_STORE barrier is executed as part of the volatile write. */
initialized = true;
}
}