Add checks whether the GlobalVariableStorage is still valid

* Add a convenience GlobalVariableReader class for CoreLibrary.

Author: eregon

src/main/java/org/truffleruby/core/CoreLibrary.java

@@ -57,7 +57,7 @@
 import org.truffleruby.language.control.JavaException;
 import org.truffleruby.language.control.RaiseException;
 import org.truffleruby.language.control.TruffleFatalException;
-import org.truffleruby.language.globals.GlobalVariableStorage;
+import org.truffleruby.language.globals.GlobalVariableReader;
 import org.truffleruby.language.globals.GlobalVariables;
 import org.truffleruby.language.loader.CodeLoader;
 import org.truffleruby.language.loader.FileLoader;
@@ -217,12 +217,12 @@ public class CoreLibrary {
     @CompilationFinal private SharedMethodInfo kernelPublicSendInfo;
     @CompilationFinal private SharedMethodInfo truffleBootMainInfo;
 
-    @CompilationFinal private GlobalVariableStorage loadPathStorage;
-    @CompilationFinal private GlobalVariableStorage loadedFeaturesStorage;
-    @CompilationFinal private GlobalVariableStorage debugStorage;
-    @CompilationFinal private GlobalVariableStorage verboseStorage;
-    @CompilationFinal private GlobalVariableStorage stdinStorage;
-    @CompilationFinal private GlobalVariableStorage stderrStorage;
+    @CompilationFinal private GlobalVariableReader loadPathReader;
+    @CompilationFinal private GlobalVariableReader loadedFeaturesReader;
+    @CompilationFinal private GlobalVariableReader debugReader;
+    @CompilationFinal private GlobalVariableReader verboseReader;
+    @CompilationFinal private GlobalVariableReader stdinReader;
+    @CompilationFinal private GlobalVariableReader stderrReader;
 
     private final ConcurrentMap<String, Boolean> patchFiles;
 
@@ -672,13 +672,12 @@ private Object verbosityOption() {
     }
 
     private void findGlobalVariableStorage() {
-        GlobalVariables globals = globalVariables;
-        loadPathStorage = globals.getStorage("$LOAD_PATH");
-        loadedFeaturesStorage = globals.getStorage("$LOADED_FEATURES");
-        debugStorage = globals.getStorage("$DEBUG");
-        verboseStorage = globals.getStorage("$VERBOSE");
-        stdinStorage = globals.getStorage("$stdin");
-        stderrStorage = globals.getStorage("$stderr");
+        loadPathReader = globalVariables.getReader("$LOAD_PATH");
+        loadedFeaturesReader = globalVariables.getReader("$LOADED_FEATURES");
+        debugReader = globalVariables.getReader("$DEBUG");
+        verboseReader = globalVariables.getReader("$VERBOSE");
+        stdinReader = globalVariables.getReader("$stdin");
+        stderrReader = globalVariables.getReader("$stderr");
     }
 
     private void initializeConstants() {
@@ -1094,24 +1093,24 @@ public GlobalVariables getGlobalVariables() {
     }
 
     public DynamicObject getLoadPath() {
-        return (DynamicObject) loadPathStorage.getValue();
+        return (DynamicObject) loadPathReader.getValue(globalVariables);
     }
 
     public DynamicObject getLoadedFeatures() {
-        return (DynamicObject) loadedFeaturesStorage.getValue();
+        return (DynamicObject) loadedFeaturesReader.getValue(globalVariables);
     }
 
     public Object getDebug() {
-        if (debugStorage != null) {
-            return debugStorage.getValue();
+        if (debugReader != null) {
+            return debugReader.getValue(globalVariables);
         } else {
             return context.getOptions().DEBUG;
         }
     }
 
     private Object verbosity() {
-        if (verboseStorage != null) {
-            return verboseStorage.getValue();
+        if (verboseReader != null) {
+            return verboseReader.getValue(globalVariables);
         } else {
             return verbosityOption();
         }
@@ -1128,11 +1127,11 @@ public boolean isVerbose() {
     }
 
     public Object getStdin() {
-        return stdinStorage.getValue();
+        return stdinReader.getValue(globalVariables);
     }
 
     public Object getStderr() {
-        return stderrStorage.getValue();
+        return stderrReader.getValue(globalVariables);
     }
 
     public DynamicObject getMainObject() {

src/main/java/org/truffleruby/language/globals/GlobalVariableReader.java

@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. This
+ * code is released under a tri EPL/GPL/LGPL license. You can use it,
+ * redistribute it and/or modify it under the terms of the:
+ *
+ * Eclipse Public License version 1.0, or
+ * GNU General Public License version 2, or
+ * GNU Lesser General Public License version 2.1.
+ */
+package org.truffleruby.language.globals;
+
+import com.oracle.truffle.api.CompilerDirectives;
+import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
+
+public class GlobalVariableReader {
+
+    private final String name;
+    @CompilationFinal private GlobalVariableStorage storage;
+
+    GlobalVariableReader(GlobalVariables globalVariables, String name) {
+        this.name = name;
+        this.storage = globalVariables.getStorage(name);
+    }
+
+    public Object getValue(GlobalVariables globalVariables) {
+        if (!storage.getValidAssumption().isValid()) {
+            CompilerDirectives.transferToInterpreterAndInvalidate();
+            storage = globalVariables.getStorage(name);
+        }
+
+        return storage.getValue();
+    }
+
+}

src/main/java/org/truffleruby/language/globals/GlobalVariableStorage.java

@@ -12,6 +12,7 @@
 import com.oracle.truffle.api.Assumption;
 import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.object.DynamicObject;
 import com.oracle.truffle.api.utilities.CyclicAssumption;
 import org.truffleruby.RubyContext;
@@ -21,6 +22,7 @@ public class GlobalVariableStorage {
 
     private static final Object UNSET_VALUE = NotProvided.INSTANCE;
 
+    private final Assumption validAssumption = Truffle.getRuntime().createAssumption("global variable not aliased");
     private final CyclicAssumption unchangedAssumption = new CyclicAssumption("global variable unchanged");
     private int changes = 0;
 
@@ -35,21 +37,17 @@ public class GlobalVariableStorage {
     private final DynamicObject isDefined;
 
     GlobalVariableStorage(Object defaultValue, DynamicObject getter, DynamicObject setter, DynamicObject isDefined) {
-        this.defaultValue = defaultValue;
-        this.value = UNSET_VALUE;
-        this.getter = getter;
-        this.setter = setter;
-        this.isDefined = isDefined;
-        assert ((getter == null) == (setter == null)) & ((getter == null) == (isDefined == null));
+        this(UNSET_VALUE, defaultValue, getter, setter, isDefined);
     }
 
     GlobalVariableStorage(Object value, Object defaultValue, DynamicObject getter, DynamicObject setter, DynamicObject isDefined) {
+        assert ((getter == null) == (setter == null)) & ((getter == null) == (isDefined == null));
+
         this.defaultValue = defaultValue;
         this.value = value;
         this.getter = getter;
         this.setter = setter;
         this.isDefined = isDefined;
-        assert ((getter == null) == (setter == null)) & ((getter == null) == (isDefined == null));
     }
 
     public Object getValue() {
@@ -81,6 +79,10 @@ public DynamicObject getIsDefined() {
         return isDefined;
     }
 
+    public Assumption getValidAssumption() {
+        return validAssumption;
+    }
+
     public Assumption getUnchangedAssumption() {
         return unchangedAssumption.getAssumption();
     }

src/main/java/org/truffleruby/language/globals/GlobalVariables.java

@@ -34,13 +34,23 @@ public String getOriginalName(String name) {
         return aliases.getOrDefault(name, name);
     }
 
+    /**
+     * The returned storage must be checked if it is still valid with
+     * {@link GlobalVariableStorage#getValidAssumption()}. A storage
+     * becomes invalid when it is aliased and therefore the storage
+     * instance needs to change.
+     */
     @TruffleBoundary
     public GlobalVariableStorage getStorage(String name) {
         final String originalName = getOriginalName(name);
         return ConcurrentOperations.getOrCompute(variables, originalName,
                 k -> new GlobalVariableStorage(defaultValue, null, null, null));
     }
 
+    public GlobalVariableReader getReader(String name) {
+        return new GlobalVariableReader(this, name);
+    }
+
     public GlobalVariableStorage put(String name, Object value) {
         assert !variables.containsKey(name);
         final GlobalVariableStorage storage = new GlobalVariableStorage(value, defaultValue, null, null, null);
@@ -62,6 +72,7 @@ public boolean contains(String name) {
 
     @TruffleBoundary
     public void alias(String oldName, String newName) {
+        // TODO
         // Record an alias of an alias against the original.
         oldName = aliases.getOrDefault(oldName, oldName);
         aliases.put(newName, oldName);

src/main/java/org/truffleruby/language/globals/IsDefinedGlobalVariableNode.java

@@ -32,7 +32,7 @@ public IsDefinedGlobalVariableNode(String name) {
 
     public abstract Object executeIsDefined(VirtualFrame frame);
 
-    @Specialization(guards = "storage.isSimple()")
+    @Specialization(guards = "storage.isSimple()", assumptions = "storage.getValidAssumption()")
     public Object executeDefined(
             @Cached("getStorage()") GlobalVariableStorage storage) {
         if (storage.isDefined()) {
@@ -42,15 +42,15 @@ public Object executeDefined(
         }
     }
 
-    @Specialization(guards = { "storage.hasHooks()", "arity == 0" })
+    @Specialization(guards = { "storage.hasHooks()", "arity == 0" }, assumptions = "storage.getValidAssumption()")
     public Object executeDefinedHooks(
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("isDefinedArity(storage)") int arity,
             @Cached("new()") YieldNode yieldNode) {
         return yieldNode.dispatch(storage.getIsDefined());
     }
 
-    @Specialization(guards = { "storage.hasHooks()", "arity == 1" })
+    @Specialization(guards = { "storage.hasHooks()", "arity == 1" }, assumptions = "storage.getValidAssumption()")
     public Object executeDefinedHooksWithBinding(VirtualFrame frame,
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("isDefinedArity(storage)") int arity,

src/main/java/org/truffleruby/language/globals/ReadGlobalVariableNode.java

@@ -28,22 +28,22 @@ public ReadGlobalVariableNode(String name) {
         this.name = name;
     }
 
-    @Specialization(guards = "storage.isSimple()")
+    @Specialization(guards = "storage.isSimple()", assumptions = "storage.getValidAssumption()")
     public Object read(
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("create(name)") ReadSimpleGlobalVariableNode simpleNode) {
         return simpleNode.execute();
     }
 
-    @Specialization(guards = { "storage.hasHooks()", "arity == 0" })
+    @Specialization(guards = { "storage.hasHooks()", "arity == 0" }, assumptions = "storage.getValidAssumption()")
     public Object readHooks(VirtualFrame frame,
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("getterArity(storage)") int arity,
             @Cached("new()") YieldNode yieldNode) {
         return yieldNode.dispatch(storage.getGetter());
     }
 
-    @Specialization(guards = { "storage.hasHooks()", "arity == 1" })
+    @Specialization(guards = { "storage.hasHooks()", "arity == 1" }, assumptions = "storage.getValidAssumption()")
     public Object readHooksWithBinding(VirtualFrame frame,
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("getterArity(storage)") int arity,

src/main/java/org/truffleruby/language/globals/ReadSimpleGlobalVariableNode.java

@@ -19,14 +19,14 @@ public ReadSimpleGlobalVariableNode(String name) {
 
     public abstract Object execute();
 
-    @Specialization(assumptions = "storage.getUnchangedAssumption()")
+    @Specialization(assumptions = { "storage.getUnchangedAssumption()", "storage.getValidAssumption()" })
     public Object readConstant(
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("storage.getValue()") Object value) {
         return value;
     }
 
-    @Specialization
+    @Specialization(assumptions = "storage.getValidAssumption()")
     public Object read(
             @Cached("getStorage()") GlobalVariableStorage storage) {
         return storage.getValue();

src/main/java/org/truffleruby/language/globals/WriteGlobalVariableNode.java

@@ -28,15 +28,15 @@ public WriteGlobalVariableNode(String name) {
         this.name = name;
     }
 
-    @Specialization(guards = "storage.isSimple()")
+    @Specialization(guards = "storage.isSimple()", assumptions = "storage.getValidAssumption()")
     public Object write(VirtualFrame frame, Object value,
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("create(name)") WriteSimpleGlobalVariableNode simpleNode) {
         simpleNode.execute(value);
         return value;
     }
 
-    @Specialization(guards = { "storage.hasHooks()", "arity != 2" })
+    @Specialization(guards = { "storage.hasHooks()", "arity != 2" }, assumptions = "storage.getValidAssumption()")
     public Object writeHooks(VirtualFrame frame, Object value,
                              @Cached("getStorage()") GlobalVariableStorage storage,
                              @Cached("setterArity(storage)") int arity,
@@ -45,7 +45,7 @@ public Object writeHooks(VirtualFrame frame, Object value,
         return value;
     }
 
-    @Specialization(guards = { "storage.hasHooks()", "arity == 2" })
+    @Specialization(guards = { "storage.hasHooks()", "arity == 2" }, assumptions = "storage.getValidAssumption()")
     public Object writeHooksWithBinding(VirtualFrame frame, Object value,
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("setterArity(storage)") int arity,

src/main/java/org/truffleruby/language/globals/WriteSimpleGlobalVariableNode.java

@@ -25,7 +25,8 @@ public WriteSimpleGlobalVariableNode(String name) {
 
     public abstract Object execute(Object value);
 
-    @Specialization(guards = "referenceEqualNode.executeReferenceEqual(value, previousValue)", assumptions = "storage.getUnchangedAssumption()")
+    @Specialization(guards = "referenceEqualNode.executeReferenceEqual(value, previousValue)",
+                    assumptions = { "storage.getUnchangedAssumption()", "storage.getValidAssumption()" })
     public Object writeTryToKeepConstant(Object value,
             @Cached("getStorage()") GlobalVariableStorage storage,
             @Cached("storage.getValue()") Object previousValue) {
@@ -35,7 +36,8 @@ public Object writeTryToKeepConstant(Object value,
         return previousValue;
     }
 
-    @Specialization(guards = "storage.isAssumeConstant()", assumptions = "storage.getUnchangedAssumption()")
+    @Specialization(guards = "storage.isAssumeConstant()",
+                    assumptions = { "storage.getUnchangedAssumption()", "storage.getValidAssumption()" })
     public Object writeAssumeConstant(Object value,
             @Cached("getStorage()") GlobalVariableStorage storage) {
         if (getContext().getSharedObjects().isSharing()) {
@@ -46,7 +48,7 @@ public Object writeAssumeConstant(Object value,
         return value;
     }
 
-    @Specialization(guards = "!storage.isAssumeConstant()", replaces = "writeAssumeConstant")
+    @Specialization(guards = "!storage.isAssumeConstant()", assumptions = "storage.getValidAssumption()", replaces = "writeAssumeConstant")
     public Object write(Object value,
             @Cached("getStorage()") GlobalVariableStorage storage) {
         if (getContext().getSharedObjects().isSharing()) {