diff --git a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java index 1e3fe8cc631d..e42f6270181f 100644 --- a/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java +++ b/substratevm/src/com.oracle.svm.core/src/com/oracle/svm/core/jdk/SystemPropertiesSupport.java @@ -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; @@ -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. @@ -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; } @@ -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); } } @@ -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; } } 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 15d7613a9cdf..30c10d195796 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 @@ -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. */ @@ -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() { @@ -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() { @@ -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; } }