[GR-14095] Fixes

PullRequest: truffleruby/646

Author: chrisseaton

src/launcher/java/org/truffleruby/launcher/RubyLauncher.java

@@ -222,6 +222,8 @@ private int runRubyMain(Context.Builder contextBuilder, CommandLineOptions confi
 
     private static void debugPreInitialization() {
         if (!isAOT() && TruffleRuby.PRE_INITIALIZE_CONTEXTS) {
+            // This is only run when saying that you are pre-initialising a context but actually you're not running in the image generator
+
             try {
                 final Class<?> holderClz = Class.forName("org.graalvm.polyglot.Engine$ImplHolder");
                 final Method preInitMethod = holderClz.getDeclaredMethod("preInitializeEngine");

src/main/java/org/truffleruby/cext/WrapNode.java

@@ -19,6 +19,7 @@
 import org.jcodings.specific.UTF8Encoding;
 import org.truffleruby.Layouts;
 import org.truffleruby.core.rope.RopeOperations;
+import org.truffleruby.extra.ffi.Pointer;
 import org.truffleruby.language.NotProvided;
 import org.truffleruby.language.RubyBaseNode;
 import org.truffleruby.language.control.RaiseException;
@@ -85,7 +86,12 @@ public ValueWrapper wrapValue(DynamicObject value,
             synchronized (value) {
                 wrapper = (ValueWrapper) readWrapperNode.execute(value);
                 if (wrapper == null) {
+                    /*
+                     * This is double-checked locking, but it's safe because the object that we create,
+                     * the ValueWrapper, is not published until after a memory store fence.
+                     */
                     wrapper = new ValueWrapper(value, UNSET_HANDLE);
+                    Pointer.UNSAFE.storeFence();
                     writeWrapperNode.write(value, wrapper);
                 }
             }

src/main/java/org/truffleruby/core/module/ModuleOperations.java

@@ -543,6 +543,13 @@ public static void setClassVariable(final RubyContext context, DynamicObject mod
         if (!trySetClassVariable(module, name, value)) {
             synchronized (context.getClassVariableDefinitionLock()) {
                 if (!trySetClassVariable(module, name, value)) {
+                    /*
+                     * This is double-checked locking, but it is safe because when writing to a ConcurrentHashMap "an
+                     * update operation for a given key bears a happens-before relation with any (non-null) retrieval
+                     * for that key reporting the updated value" (JavaDoc) so the value is guaranteed to be fully
+                     * published before it can be found in the map.
+                     */
+
                     moduleFields.getClassVariables().put(name, value);
                 }
             }

src/main/java/org/truffleruby/core/rope/PathToRopeCache.java

@@ -15,6 +15,7 @@
 import org.truffleruby.core.string.StringOperations;
 
 import java.util.WeakHashMap;
+import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
@@ -40,23 +41,27 @@ public PathToRopeCache(RubyContext context) {
      */
     @TruffleBoundary
     public Rope getCachedPath(String string) {
-        lock.readLock().lock();
+        final Lock readLock = lock.readLock();
+
+        readLock.lock();
         try {
             final Rope rope = javaStringToRope.get(string);
             if (rope != null) {
                 return rope;
             }
         } finally {
-            lock.readLock().unlock();
+            readLock.unlock();
         }
 
         final Rope cachedRope = context.getRopeCache().getRope(StringOperations.encodeRope(string, UTF8Encoding.INSTANCE));
 
-        lock.writeLock().lock();
+        final Lock writeLock = lock.writeLock();
+
+        writeLock.lock();
         try {
             javaStringToRope.putIfAbsent(string, cachedRope);
         } finally {
-            lock.writeLock().unlock();
+            writeLock.unlock();
         }
 
         return cachedRope;

src/main/java/org/truffleruby/core/symbol/SymbolTable.java

@@ -35,6 +35,7 @@
 import java.util.Collection;
 import java.util.Map;
 import java.util.WeakHashMap;
+import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReadWriteLock;
 import java.util.concurrent.locks.ReentrantReadWriteLock;
 
@@ -65,14 +66,16 @@ public DynamicObject getSymbol(String string) {
         final StringKey stringKey = new StringKey(string, hashing);
         DynamicObject symbol;
 
-        lock.readLock().lock();
+        final Lock readLock = lock.readLock();
+
+        readLock.lock();
         try {
             symbol = lookupCache(stringToSymbolCache, stringKey);
             if (symbol != null) {
                 return symbol;
             }
         } finally {
-            lock.readLock().unlock();
+            readLock.unlock();
         }
 
         final Rope rope;
@@ -84,13 +87,16 @@ public DynamicObject getSymbol(String string) {
         symbol = getSymbol(rope);
 
         // Add it to the direct j.l.String to Symbol cache
-        lock.writeLock().lock();
+
+        final Lock writeLock = lock.writeLock();
+
+        writeLock.lock();
         try {
             if (lookupCache(stringToSymbolCache, stringKey) == null) {
                 stringToSymbolCache.put(stringKey, new SoftReference<>(symbol));
             }
         } finally {
-            lock.writeLock().unlock();
+            writeLock.unlock();
         }
 
         return symbol;

src/main/java/org/truffleruby/language/loader/ReentrantLockFreeingMap.java

@@ -7,7 +7,6 @@
  * GNU General Public License version 2, or
  * GNU Lesser General Public License version 2.1.
  */
-
 package org.truffleruby.language.loader;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;

src/main/java/org/truffleruby/language/loader/ResourceLoader.java

@@ -46,13 +46,15 @@ public RubySource loadResource(String path, boolean internal) throws IOException
             throw new FileNotFoundException(path);
         }
 
-        // We guarantee that we only put UTF-8 source files into resources
-        final InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8);
+        final Source source;
 
-        final Source source = Source
-                .newBuilder(TruffleRuby.LANGUAGE_ID, reader, path)
-                .internal(internal)
-                .build();
+        // We guarantee that we only put UTF-8 source files into resources
+        try (final InputStreamReader reader = new InputStreamReader(stream, StandardCharsets.UTF_8)) {
+            source = Source
+                    .newBuilder(TruffleRuby.LANGUAGE_ID, reader, path)
+                    .internal(internal)
+                    .build();
+        }
 
         return new RubySource(source);
     }

src/shared/java/org/truffleruby/shared/BasicPlatform.java

@@ -62,7 +62,7 @@ public static String getOSName() {
     public static OS_TYPE determineOS() {
         final String osName = System.getProperty("os.name");
 
-        final String lowerOSName = osName.toLowerCase();
+        final String lowerOSName = osName.toLowerCase(Locale.ENGLISH);
         if (lowerOSName.contains("windows")) {
             return OS_TYPE.WINDOWS;
         }