[GR-29429] Let startAutoLoad() enter the safepoint when acquiring the lock

PullRequest: truffleruby/2425

Author: eregon

spec/ruby/fixtures/code/concurrent.rb

@@ -2,7 +2,7 @@
 Thread.current[:in_concurrent_rb] = true
 
 if t = Thread.current[:wait_for]
-  Thread.pass until t.backtrace && t.backtrace.any? { |call| call.include? 'require' }
+  Thread.pass until t.backtrace && t.backtrace.any? { |call| call.include? 'require' } && t.stop?
 end
 
 if Thread.current[:con_raise]

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

@@ -374,7 +374,7 @@ public void setAutoloadConstant(RubyContext context, Node currentNode, String na
         if (lock.isLocked()) {
             // We need to handle the new autoload constant immediately
             // if Object.autoload(name, filename) is executed from filename.rb
-            GetConstantNode.autoloadConstantStart(autoloadConstant);
+            GetConstantNode.autoloadConstantStart(context, autoloadConstant, currentNode);
         }
 
         context.getFeatureLoader().addAutoload(autoloadConstant);

src/main/java/org/truffleruby/core/mutex/MutexOperations.java

@@ -32,7 +32,7 @@ protected static void lock(RubyContext context, ReentrantLock lock, RubyThread t
     }
 
     @TruffleBoundary
-    public static void lockInternal(RubyContext context, ReentrantLock lock, RubyNode currentNode) {
+    public static void lockInternal(RubyContext context, ReentrantLock lock, Node currentNode) {
         if (lock.tryLock()) {
             return;
         }

src/main/java/org/truffleruby/language/AutoloadConstant.java

@@ -12,6 +12,9 @@
 import java.util.concurrent.locks.ReentrantLock;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
+import com.oracle.truffle.api.nodes.Node;
+import org.truffleruby.RubyContext;
+import org.truffleruby.core.mutex.MutexOperations;
 import org.truffleruby.language.library.RubyStringLibrary;
 
 public class AutoloadConstant {
@@ -44,8 +47,8 @@ private ReentrantLock getAutoloadLock() {
     }
 
     @TruffleBoundary
-    public void startAutoLoad() {
-        getAutoloadLock().lock();
+    public void startAutoLoad(RubyContext context, Node currentNode) {
+        MutexOperations.lockInternal(context, getAutoloadLock(), currentNode);
     }
 
     @TruffleBoundary

src/main/java/org/truffleruby/language/constants/GetConstantNode.java

@@ -9,6 +9,7 @@
  */
 package org.truffleruby.language.constants;
 
+import com.oracle.truffle.api.nodes.Node;
 import org.truffleruby.RubyContext;
 import org.truffleruby.RubyLanguage;
 import org.truffleruby.SuppressFBWarnings;
@@ -118,7 +119,7 @@ protected Object autoloadConstant(
         }
 
         // Mark the autoload constant as loading already here and not in RequireNode so that recursive lookups act as "being loaded"
-        autoloadConstantStart(autoloadConstant);
+        autoloadConstantStart(getContext(), autoloadConstant, this);
         try {
             callRequireNode.call(coreLibrary().mainObject, "require", feature);
 
@@ -130,8 +131,8 @@ protected Object autoloadConstant(
     }
 
     @TruffleBoundary
-    public static void autoloadConstantStart(RubyConstant autoloadConstant) {
-        autoloadConstant.getAutoloadConstant().startAutoLoad();
+    public static void autoloadConstantStart(RubyContext context, RubyConstant autoloadConstant, Node currentNode) {
+        autoloadConstant.getAutoloadConstant().startAutoLoad(context, currentNode);
 
         // We need to notify cached lookup that we are autoloading the constant, as constant
         // lookup changes based on whether an autoload constant is loading or not (constant

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

@@ -12,7 +12,8 @@
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.concurrent.locks.ReentrantLock;
 
-import org.truffleruby.core.thread.ThreadManager;
+import org.truffleruby.RubyContext;
+import org.truffleruby.core.mutex.MutexOperations;
 
 import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
 import com.oracle.truffle.api.nodes.Node;
@@ -68,19 +69,9 @@ public boolean isCurrentThreadHoldingLock(K key) {
     }
 
     @TruffleBoundary
-    public boolean lock(
-            Node currentNode,
-            ThreadManager threadManager,
-            K key,
-            final ReentrantLock lock) {
-
+    public boolean lock(RubyContext context, K key, ReentrantLock lock, Node currentNode) {
         // Also sets status to sleep in MRI
-        threadManager.runUntilResult(
-                currentNode,
-                () -> {
-                    lock.lockInterruptibly();
-                    return ThreadManager.BlockingAction.SUCCESS;
-                });
+        MutexOperations.lockInternal(context, lock, currentNode);
         // ensure that we are not holding removed lock
         if (lock == locks.get(key)) {
             return true;

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

@@ -117,7 +117,7 @@ private boolean requireConsideringAutoload(String feature, String expandedPath,
             }
 
             for (RubyConstant constant : toAutoload) {
-                GetConstantNode.autoloadConstantStart(constant);
+                GetConstantNode.autoloadConstantStart(getContext(), constant, this);
             }
         }
 
@@ -168,7 +168,7 @@ private boolean doRequire(String originalFeature, String expandedPath, Object pa
                 }
             }
 
-            if (!fileLocks.lock(this, getContext().getThreadManager(), expandedPath, lock)) {
+            if (!fileLocks.lock(getContext(), expandedPath, lock, this)) {
                 continue;
             }