Skip to content

Commit 6bd14d8

Browse files
eregoneregon
committed
Trigger autoload when requiring an autoload feature
* Add name in RubyConstant which helps debugging and passing constant names around.
1 parent 5b3d0c9 commit 6bd14d8

File tree

8 files changed

+140
-53
lines changed

8 files changed

+140
-53
lines changed

‎spec/tags/core/module/autoload_tags.txt

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1 @@
11
fails:Module#autoload (concurrently) blocks others threads while doing an autoload
2-
fails:Module#autoload the autoload is triggered when the same file is required directly with a full path
3-
fails:Module#autoload the autoload is triggered when the same file is required directly with a relative path
4-
fails:Module#autoload the autoload is triggered when the same file is required directly in a nested require

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

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -321,12 +321,13 @@ public RubyConstant setConstant(RubyContext context, Node currentNode, String na
321321
@TruffleBoundary
322322
public void setAutoloadConstant(RubyContext context, Node currentNode, String name, DynamicObject filename) {
323323
assert RubyGuards.isRubyString(filename);
324+
RubyConstant autoloadConstant = setConstantInternal(context, currentNode, name, filename, true);
324325
if (context.getOptions().LOG_AUTOLOAD) {
325-
RubyLanguage.LOGGER.info(() -> String.format("%s: setting up autoload %s::%s with %s",
326+
RubyLanguage.LOGGER.info(() -> String.format("%s: setting up autoload %s with %s",
326327
context.fileLine(context.getCallStack().getTopMostUserSourceSection()),
327-
getName(), name, filename));
328+
autoloadConstant, filename));
328329
}
329-
setConstantInternal(context, currentNode, name, filename, true);
330+
context.getFeatureLoader().addAutoload(autoloadConstant);
330331
}
331332

332333
private RubyConstant setConstantInternal(RubyContext context, Node currentNode, String name, Object value, boolean autoload) {
@@ -338,18 +339,18 @@ private RubyConstant setConstantInternal(RubyContext context, Node currentNode,
338339
RubyConstant newConstant;
339340
do {
340341
previous = constants.get(name);
341-
newConstant = newConstant(currentNode, value, autoload, previous);
342+
newConstant = newConstant(currentNode, name, value, autoload, previous);
342343
} while (!ConcurrentOperations.replace(constants, name, previous, newConstant));
343344

344345
newConstantsVersion();
345-
return previous;
346+
return autoload ? newConstant : previous;
346347
}
347348

348-
private RubyConstant newConstant(Node currentNode, Object value, boolean autoload, RubyConstant previous) {
349+
private RubyConstant newConstant(Node currentNode, String name, Object value, boolean autoload, RubyConstant previous) {
349350
final boolean isPrivate = previous != null && previous.isPrivate();
350351
final boolean isDeprecated = previous != null && previous.isDeprecated();
351352
final SourceSection sourceSection = currentNode != null ? currentNode.getSourceSection() : null;
352-
return new RubyConstant(rubyModuleObject, value, isPrivate, autoload, isDeprecated, sourceSection);
353+
return new RubyConstant(rubyModuleObject, name, value, isPrivate, autoload, isDeprecated, sourceSection);
353354
}
354355

355356
@TruffleBoundary

‎src/main/java/org/truffleruby/language/RubyConstant.java

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
public class RubyConstant implements ObjectGraphNode {
2424

2525
private final DynamicObject declaringModule;
26+
private final String name;
2627
private final Object value;
2728
private final boolean isPrivate;
2829
private final boolean isDeprecated;
@@ -34,13 +35,14 @@ public class RubyConstant implements ObjectGraphNode {
3435

3536
private final SourceSection sourceSection;
3637

37-
public RubyConstant(DynamicObject declaringModule, Object value, boolean isPrivate, boolean autoload, boolean isDeprecated, SourceSection sourceSection) {
38-
this(declaringModule, value, isPrivate, autoload, false, isDeprecated, sourceSection);
38+
public RubyConstant(DynamicObject declaringModule, String name, Object value, boolean isPrivate, boolean autoload, boolean isDeprecated, SourceSection sourceSection) {
39+
this(declaringModule, name, value, isPrivate, autoload, false, isDeprecated, sourceSection);
3940
}
4041

41-
private RubyConstant(DynamicObject declaringModule, Object value, boolean isPrivate, boolean autoload, boolean undefined, boolean isDeprecated, SourceSection sourceSection) {
42+
private RubyConstant(DynamicObject declaringModule, String name, Object value, boolean isPrivate, boolean autoload, boolean undefined, boolean isDeprecated, SourceSection sourceSection) {
4243
assert RubyGuards.isRubyModule(declaringModule);
4344
this.declaringModule = declaringModule;
45+
this.name = name;
4446
this.value = value;
4547
this.isPrivate = isPrivate;
4648
this.isDeprecated = isDeprecated;
@@ -53,6 +55,10 @@ public DynamicObject getDeclaringModule() {
5355
return declaringModule;
5456
}
5557

58+
public String getName() {
59+
return name;
60+
}
61+
5662
public boolean hasValue() {
5763
return !autoload && !undefined;
5864
}
@@ -78,21 +84,21 @@ public RubyConstant withPrivate(boolean isPrivate) {
7884
if (isPrivate == this.isPrivate) {
7985
return this;
8086
} else {
81-
return new RubyConstant(declaringModule, value, isPrivate, autoload, undefined, isDeprecated, sourceSection);
87+
return new RubyConstant(declaringModule, name, value, isPrivate, autoload, undefined, isDeprecated, sourceSection);
8288
}
8389
}
8490

8591
public RubyConstant withDeprecated() {
8692
if (this.isDeprecated()) {
8793
return this;
8894
} else {
89-
return new RubyConstant(declaringModule, value, isPrivate, autoload, undefined, true, sourceSection);
95+
return new RubyConstant(declaringModule, name, value, isPrivate, autoload, undefined, true, sourceSection);
9096
}
9197
}
9298

9399
public RubyConstant undefined() {
94100
assert autoload;
95-
return new RubyConstant(declaringModule, null, isPrivate, false, true, isDeprecated, sourceSection);
101+
return new RubyConstant(declaringModule, name, null, isPrivate, false, true, isDeprecated, sourceSection);
96102
}
97103

98104
@TruffleBoundary
@@ -180,4 +186,9 @@ public void getAdjacentObjects(Set<DynamicObject> adjacent) {
180186
}
181187
}
182188

189+
@Override
190+
public String toString() {
191+
return Layouts.MODULE.getFields(getDeclaringModule()).getName() + "::" + getName();
192+
}
193+
183194
}

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

Lines changed: 13 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,10 @@
2020
import org.truffleruby.RubyLanguage;
2121
import org.truffleruby.core.module.ModuleFields;
2222
import org.truffleruby.core.module.ModuleOperations;
23-
import org.truffleruby.core.string.StringOperations;
2423
import org.truffleruby.language.LexicalScope;
2524
import org.truffleruby.language.RubyBaseNode;
2625
import org.truffleruby.language.RubyConstant;
2726
import org.truffleruby.language.dispatch.CallDispatchHeadNode;
28-
import org.truffleruby.language.loader.FeatureLoader;
2927

3028
public abstract class GetConstantNode extends RubyBaseNode {
3129

@@ -63,44 +61,36 @@ protected Object autoloadConstant(LexicalScope lexicalScope, DynamicObject modul
6361
@Cached("createPrivate()") CallDispatchHeadNode callRequireNode) {
6462

6563
final DynamicObject feature = autoloadConstant.getAutoloadPath();
66-
final DynamicObject autoloadConstantModule = autoloadConstant.getDeclaringModule();
67-
final ModuleFields fields = Layouts.MODULE.getFields(autoloadConstantModule);
6864

6965
if (autoloadConstant.isAutoloadingThread()) {
7066
// Pretend the constant does not exist while it is autoloading
7167
return doMissingConstant(module, name, getSymbol(name));
7268
}
7369

74-
final FeatureLoader featureLoader = getContext().getFeatureLoader();
75-
final String expandedPath = featureLoader.findFeature(StringOperations.getString(feature));
76-
if (expandedPath != null && featureLoader.getFileLocks().isCurrentThreadHoldingLock(expandedPath)) {
77-
// We found an autoload constant while we are already require-ing the autoload file,
78-
// consider it missing to avoid circular require warnings and calling #require twice.
79-
// For instance, autoload :RbConfig, "rbconfig"; require "rbconfig" causes this.
80-
if (getContext().getOptions().LOG_AUTOLOAD) {
81-
RubyLanguage.LOGGER.info(() -> String.format("%s: %s::%s is being treated as missing while loading %s",
82-
getContext().fileLine(getContext().getCallStack().getTopMostUserSourceSection()),
83-
Layouts.MODULE.getFields(module).getName(), name, expandedPath));
84-
}
85-
return doMissingConstant(module, name, getSymbol(name));
86-
}
87-
88-
autoloadConstant.startAutoLoad();
89-
9070
if (getContext().getOptions().LOG_AUTOLOAD) {
91-
RubyLanguage.LOGGER.info(() -> String.format("%s: autoloading %s::%s with %s",
71+
RubyLanguage.LOGGER.info(() -> String.format("%s: autoloading %s with %s",
9272
getContext().fileLine(getContext().getCallStack().getTopMostUserSourceSection()),
93-
Layouts.MODULE.getFields(autoloadConstantModule).getName(), name, autoloadConstant.getAutoloadPath()));
73+
autoloadConstant, autoloadConstant.getAutoloadPath()));
9474
}
9575

76+
final Runnable require = () -> callRequireNode.call(coreLibrary().getMainObject(), "require", feature);
77+
return autoloadConstant(lexicalScope, module, name, autoloadConstant, lookupConstantNode, require);
78+
}
79+
80+
@TruffleBoundary
81+
public Object autoloadConstant(LexicalScope lexicalScope, DynamicObject module, String name, RubyConstant autoloadConstant, LookupConstantInterface lookupConstantNode, Runnable require) {
82+
final DynamicObject autoloadConstantModule = autoloadConstant.getDeclaringModule();
83+
final ModuleFields fields = Layouts.MODULE.getFields(autoloadConstantModule);
84+
85+
autoloadConstant.startAutoLoad();
9686
try {
9787

9888
// We need to notify cached lookup that we are autoloading the constant, as constant
9989
// lookup changes based on whether an autoload constant is loading or not (constant
10090
// lookup ignores being-autoloaded constants).
10191
fields.newConstantsVersion();
10292

103-
callRequireNode.call(coreLibrary().getMainObject(), "require", feature);
93+
require.run();
10494

10595
RubyConstant resolvedConstant = lookupConstantNode.lookupConstant(lexicalScope, module, name);
10696

@@ -122,7 +112,6 @@ protected Object autoloadConstant(LexicalScope lexicalScope, DynamicObject modul
122112
} finally {
123113
autoloadConstant.stopAutoLoad();
124114
}
125-
126115
}
127116

128117
@Specialization(

‎src/main/java/org/truffleruby/language/constants/WriteConstantNode.java

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import com.oracle.truffle.api.source.SourceSection;
1717
import org.truffleruby.Layouts;
1818
import org.truffleruby.core.constant.WarnAlreadyInitializedNode;
19-
import org.truffleruby.core.module.ModuleOperations;
2019
import org.truffleruby.language.RubyConstant;
2120
import org.truffleruby.language.RubyGuards;
2221
import org.truffleruby.language.RubyNode;

‎src/main/java/org/truffleruby/language/loader/FeatureLoader.java

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
import org.truffleruby.core.string.StringOperations;
3232
import org.truffleruby.extra.TruffleRubyNodes;
3333
import org.truffleruby.extra.ffi.Pointer;
34+
import org.truffleruby.language.RubyConstant;
3435
import org.truffleruby.language.control.JavaException;
3536
import org.truffleruby.language.control.RaiseException;
3637
import org.truffleruby.shared.Metrics;
@@ -47,12 +48,19 @@
4748
import java.util.HashMap;
4849
import java.util.List;
4950
import java.util.Map;
51+
import java.util.concurrent.locks.ReentrantLock;
5052

5153
public class FeatureLoader {
5254

5355
private final RubyContext context;
5456

5557
private final ReentrantLockFreeingMap<String> fileLocks = new ReentrantLockFreeingMap<>();
58+
/**
59+
* Maps basename without extension -> autoload path -> autoload constant,
60+
* to detect when require-ing a file already registered with autoload.
61+
*/
62+
private final Map<String, Map<String, RubyConstant>> registeredAutoloads = new HashMap<>();
63+
private final ReentrantLock registeredAutoloadsLock = new ReentrantLock();
5664

5765
private final Object cextImplementationLock = new Object();
5866
private boolean cextImplementationLoaded = false;
@@ -72,6 +80,52 @@ public void initialize(NativeConfiguration nativeConfiguration, TruffleNFIPlatfo
7280
}
7381
}
7482

83+
public void addAutoload(RubyConstant autoloadConstant) {
84+
final String basename = basenameWithoutExtension(StringOperations.getString(autoloadConstant.getAutoloadPath()));
85+
86+
registeredAutoloadsLock.lock();
87+
try {
88+
final Map<String, RubyConstant> constants = registeredAutoloads.computeIfAbsent(basename, k -> new HashMap<>());
89+
constants.put(StringOperations.getString(autoloadConstant.getAutoloadPath()), autoloadConstant);
90+
} finally {
91+
registeredAutoloadsLock.unlock();
92+
}
93+
}
94+
95+
public RubyConstant isAutoloadPath(String expandedPath) {
96+
final String basename = basenameWithoutExtension(expandedPath);
97+
final RubyConstant[] constants;
98+
99+
registeredAutoloadsLock.lock();
100+
try {
101+
final Map<String, RubyConstant> constantsMap = registeredAutoloads.get(basename);
102+
if (constantsMap == null) {
103+
return null;
104+
}
105+
constants = constantsMap.values().toArray(new RubyConstant[0]);
106+
} finally {
107+
registeredAutoloadsLock.unlock();
108+
}
109+
110+
for (RubyConstant constant : constants) {
111+
final String expandedAutoloadPath = findFeature(StringOperations.getString(constant.getAutoloadPath()));
112+
if (expandedPath.equals(expandedAutoloadPath)) {
113+
return constant;
114+
}
115+
}
116+
return null;
117+
}
118+
119+
private String basenameWithoutExtension(String path) {
120+
final String basename = new File(path).getName();
121+
int i = basename.lastIndexOf('.');
122+
if (i >= 0) {
123+
return basename.substring(0, i);
124+
} else {
125+
return basename;
126+
}
127+
}
128+
75129
public String getWorkingDirectory() {
76130
final TruffleNFIPlatform nfi = context.getTruffleNFI();
77131
if (nfi == null) {

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

Lines changed: 47 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,6 @@
99
*/
1010
package org.truffleruby.language.loader;
1111

12-
import java.io.File;
13-
import java.io.IOException;
14-
import java.nio.file.Path;
15-
import java.nio.file.Paths;
16-
import java.util.List;
17-
import java.util.concurrent.ConcurrentMap;
18-
import java.util.concurrent.locks.ReentrantLock;
19-
2012
import com.oracle.truffle.api.CompilerDirectives;
2113
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
2214
import com.oracle.truffle.api.dsl.Cached;
@@ -39,9 +31,13 @@
3931
import org.truffleruby.core.cast.BooleanCastNode;
4032
import org.truffleruby.core.rope.CodeRange;
4133
import org.truffleruby.core.string.StringNodes;
34+
import org.truffleruby.language.LexicalScope;
4235
import org.truffleruby.language.RubyBaseNode;
36+
import org.truffleruby.language.RubyConstant;
4337
import org.truffleruby.language.RubyRootNode;
4438
import org.truffleruby.language.WarningNode;
39+
import org.truffleruby.language.constants.GetConstantNode;
40+
import org.truffleruby.language.constants.LookupConstantNode;
4541
import org.truffleruby.language.control.JavaException;
4642
import org.truffleruby.language.control.RaiseException;
4743
import org.truffleruby.language.dispatch.CallDispatchHeadNode;
@@ -51,6 +47,14 @@
5147
import org.truffleruby.shared.Metrics;
5248
import org.truffleruby.shared.TruffleRuby;
5349

50+
import java.io.File;
51+
import java.io.IOException;
52+
import java.nio.file.Path;
53+
import java.nio.file.Paths;
54+
import java.util.List;
55+
import java.util.concurrent.ConcurrentMap;
56+
import java.util.concurrent.locks.ReentrantLock;
57+
5458
public abstract class RequireNode extends RubyBaseNode {
5559

5660
@Child private IndirectCallNode callNode = IndirectCallNode.create();
@@ -63,6 +67,9 @@ public abstract class RequireNode extends RubyBaseNode {
6367
@Child private Node executeNode = Message.EXECUTE.createNode();
6468
@Child private WarningNode warningNode;
6569

70+
@Child private GetConstantNode getConstantNode;
71+
@Child private LookupConstantNode lookupConstantNode;
72+
6673
public static RequireNode create() {
6774
return RequireNodeGen.create();
6875
}
@@ -94,16 +101,44 @@ protected boolean require(String feature,
94101
private boolean requireWithMetrics(String feature, String expandedPathRaw, DynamicObject pathString) {
95102
requireMetric("before-require-" + feature);
96103
try {
97-
return doRequire(feature, expandedPathRaw, pathString);
104+
return requireConsideringAutoload(feature, expandedPathRaw.intern(), pathString);
98105
} finally {
99106
requireMetric("after-require-" + feature);
100107
}
101108
}
102109

103-
private boolean doRequire(String feature, String expandedPathRaw, DynamicObject pathString) {
110+
private boolean requireConsideringAutoload(String feature, String expandedPath, DynamicObject pathString) {
104111
final FeatureLoader featureLoader = getContext().getFeatureLoader();
105-
final ReentrantLockFreeingMap<String> fileLocks = featureLoader.getFileLocks();
106-
final String expandedPath = expandedPathRaw.intern();
112+
final RubyConstant autoloadConstant = featureLoader.isAutoloadPath(expandedPath);
113+
if (autoloadConstant != null &&
114+
// Do not autoload recursively from the #require call in GetConstantNode
115+
!autoloadConstant.isAutoloading()) {
116+
if (getConstantNode == null) {
117+
CompilerDirectives.transferToInterpreterAndInvalidate();
118+
getConstantNode = insert(GetConstantNode.create());
119+
}
120+
if (lookupConstantNode == null) {
121+
CompilerDirectives.transferToInterpreterAndInvalidate();
122+
lookupConstantNode = insert(LookupConstantNode.create(true, true, true));
123+
}
124+
125+
if (getContext().getOptions().LOG_AUTOLOAD) {
126+
RubyLanguage.LOGGER.info(() -> String.format("%s: requiring %s which is registered as an autoload for %s with %s",
127+
getContext().fileLine(getContext().getCallStack().getTopMostUserSourceSection()),
128+
feature, autoloadConstant, autoloadConstant.getAutoloadPath()));
129+
}
130+
131+
boolean[] result = new boolean[1];
132+
Runnable require = () -> result[0] = doRequire(feature, expandedPath, pathString);
133+
getConstantNode.autoloadConstant(LexicalScope.IGNORE, autoloadConstant.getDeclaringModule(), autoloadConstant.getName(), autoloadConstant, lookupConstantNode, require);
134+
return result[0];
135+
} else {
136+
return doRequire(feature, expandedPath, pathString);
137+
}
138+
}
139+
140+
private boolean doRequire(String feature, String expandedPath, DynamicObject pathString) {
141+
final ReentrantLockFreeingMap<String> fileLocks = getContext().getFeatureLoader().getFileLocks();
107142
final ConcurrentMap<String, Boolean> patchFiles = getContext().getCoreLibrary().getPatchFiles();
108143
Boolean patchLoaded = patchFiles.get(feature);
109144
final boolean isPatched = patchLoaded != null;

‎test/mri/excludes/TestAutoload.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
11
exclude :test_threaded_accessing_constant, "needs investigation"
22
exclude :test_threaded_accessing_inner_constant, "needs investigation"
33
exclude :test_bug_13526, "spurious and buggy test"
4+
exclude :test_autoload_same_file, "hangs"

0 commit comments

Comments
 (0)