Skip to content

Commit 618ce28

Browse files
eregoneregon
committed
[GR-14590] Remove from the autoload map after the autoload is resolved
PullRequest: truffleruby/731
2 parents c31eb63 + acdecc0 commit 618ce28

File tree

9 files changed

+141
-65
lines changed

9 files changed

+141
-65
lines changed

‎spec/ruby/core/module/autoload_spec.rb

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,29 @@ module ModuleSpecs::Autoload
274274
end
275275
end
276276

277+
describe "after the autoload is triggered by require" do
278+
before :each do
279+
@path = tmp("autoload.rb")
280+
end
281+
282+
after :each do
283+
rm_r @path
284+
end
285+
286+
it "the mapping feature to autoload is removed, and a new autoload with the same path is considered" do
287+
ModuleSpecs::Autoload.autoload :RequireMapping1, @path
288+
touch(@path) { |f| f.puts "ModuleSpecs::Autoload::RequireMapping1 = 1" }
289+
ModuleSpecs::Autoload::RequireMapping1.should == 1
290+
291+
$LOADED_FEATURES.delete(@path)
292+
ModuleSpecs::Autoload.autoload :RequireMapping2, @path[0...-3]
293+
touch(@path) { |f| f.puts "ModuleSpecs::Autoload::RequireMapping2 = 2" }
294+
ModuleSpecs::Autoload::RequireMapping2.should == 2
295+
296+
ModuleSpecs::Autoload.send :remove_const, :RequireMapping2
297+
end
298+
end
299+
277300
describe "during the autoload before the constant is assigned" do
278301
before :each do
279302
@path = fixture(__FILE__, "autoload_during_autoload.rb")

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ public class ConstantLookupResult {
2222
@CompilationFinal(dimensions = 1) private final Assumption[] assumptions;
2323

2424
public ConstantLookupResult(RubyConstant constant, Assumption... assumptions) {
25-
assert constant == null || !constant.isAutoloadingThread();
25+
assert constant == null || !(constant.isAutoload() && constant.getAutoloadConstant().isAutoloadingThread());
2626
this.constant = constant;
2727
this.assumptions = assumptions;
2828
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -560,20 +560,20 @@ public DynamicObject autoload(DynamicObject module, String name, DynamicObject f
560560
public abstract static class IsAutoloadNode extends CoreMethodArrayArgumentsNode {
561561

562562
@Specialization(guards = "isRubySymbol(name)")
563-
public Object isAutoloadSymbol(DynamicObject module, DynamicObject name) {
563+
public DynamicObject isAutoloadSymbol(DynamicObject module, DynamicObject name) {
564564
return isAutoload(module, Layouts.SYMBOL.getString(name));
565565
}
566566

567567
@Specialization(guards = "isRubyString(name)")
568-
public Object isAutoloadString(DynamicObject module, DynamicObject name) {
568+
public DynamicObject isAutoloadString(DynamicObject module, DynamicObject name) {
569569
return isAutoload(module, StringOperations.getString(name));
570570
}
571571

572-
private Object isAutoload(DynamicObject module, String name) {
572+
private DynamicObject isAutoload(DynamicObject module, String name) {
573573
final ConstantLookupResult constant = ModuleOperations.lookupConstant(getContext(), module, name);
574574

575-
if (constant.isAutoload() && !constant.getConstant().isAutoloadingThread()) {
576-
return constant.getConstant().getAutoloadPath();
575+
if (constant.isAutoload() && !constant.getConstant().getAutoloadConstant().isAutoloadingThread()) {
576+
return constant.getConstant().getAutoloadConstant().getFeature();
577577
} else {
578578
return nil();
579579
}

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,15 +110,15 @@ public static Iterable<Entry<String, RubyConstant>> getAllConstants(DynamicObjec
110110
}
111111

112112
public static boolean isConstantDefined(RubyConstant constant) {
113-
return constant != null && !constant.isAutoloadingThread();
113+
return constant != null && !(constant.isAutoload() && constant.getAutoloadConstant().isAutoloadingThread());
114114
}
115115

116116
private static boolean isConstantDefined(RubyConstant constant, ArrayList<Assumption> assumptions) {
117117
if (constant != null) {
118-
if (constant.isAutoloading()) {
118+
if (constant.isAutoload() && constant.getAutoloadConstant().isAutoloading()) {
119119
// Cannot cache the lookup of an autoloading constant as the result depends on the calling thread
120120
assumptions.add(NeverValidAssumption.INSTANCE);
121-
return !constant.isAutoloadingThread();
121+
return !constant.getAutoloadConstant().isAutoloadingThread();
122122
} else {
123123
return true;
124124
}

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

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,65 @@
1+
/*
2+
* Copyright (c) 2019 Oracle and/or its affiliates. All rights reserved. This
3+
* code is released under a tri EPL/GPL/LGPL license. You can use it,
4+
* redistribute it and/or modify it under the terms of the:
5+
*
6+
* Eclipse Public License version 1.0, or
7+
* GNU General Public License version 2, or
8+
* GNU Lesser General Public License version 2.1.
9+
*/
10+
package org.truffleruby.language;
11+
12+
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
13+
import com.oracle.truffle.api.object.DynamicObject;
14+
import org.truffleruby.core.string.StringOperations;
15+
16+
import java.util.concurrent.locks.ReentrantLock;
17+
18+
public class AutoloadConstant {
19+
20+
private final DynamicObject feature;
21+
private final String autoloadPath;
22+
private volatile ReentrantLock autoloadLock;
23+
24+
AutoloadConstant(Object feature) {
25+
assert RubyGuards.isRubyString(feature);
26+
this.feature = (DynamicObject) feature;
27+
this.autoloadPath = StringOperations.getString(this.feature);
28+
}
29+
30+
public String getAutoloadPath() {
31+
return autoloadPath;
32+
}
33+
34+
public DynamicObject getFeature() {
35+
return feature;
36+
}
37+
38+
private ReentrantLock getAutoloadLock() {
39+
synchronized (this) {
40+
if (autoloadLock == null) {
41+
autoloadLock = new ReentrantLock();
42+
}
43+
}
44+
return autoloadLock;
45+
}
46+
47+
@TruffleBoundary
48+
public void startAutoLoad() {
49+
getAutoloadLock().lock();
50+
}
51+
52+
@TruffleBoundary
53+
public void stopAutoLoad() {
54+
getAutoloadLock().unlock();
55+
}
56+
57+
public boolean isAutoloading() {
58+
return autoloadLock != null && autoloadLock.isLocked();
59+
}
60+
61+
public boolean isAutoloadingThread() {
62+
return autoloadLock != null && autoloadLock.isHeldByCurrentThread();
63+
}
64+
65+
}

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

Lines changed: 13 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,24 @@ public class RubyConstant implements ObjectGraphNode {
2828
private final boolean isPrivate;
2929
private final boolean isDeprecated;
3030

31-
private final boolean autoload;
32-
private volatile ReentrantLock autoloadLock;
31+
private final AutoloadConstant autoloadConstant;
3332
/** A autoload constant can become "undefined" after the autoload loads the file but the constant is not defined by the file */
3433
private final boolean undefined;
3534

3635
private final SourceSection sourceSection;
3736

3837
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);
38+
this(declaringModule, name, value, isPrivate, autoload ? new AutoloadConstant(value) : null, false, isDeprecated, sourceSection);
4039
}
4140

42-
private RubyConstant(DynamicObject declaringModule, String name, Object value, boolean isPrivate, boolean autoload, boolean undefined, boolean isDeprecated, SourceSection sourceSection) {
41+
private RubyConstant(DynamicObject declaringModule, String name, Object value, boolean isPrivate, AutoloadConstant autoloadConstant, boolean undefined, boolean isDeprecated, SourceSection sourceSection) {
4342
assert RubyGuards.isRubyModule(declaringModule);
4443
this.declaringModule = declaringModule;
4544
this.name = name;
4645
this.value = value;
4746
this.isPrivate = isPrivate;
4847
this.isDeprecated = isDeprecated;
49-
this.autoload = autoload;
48+
this.autoloadConstant = autoloadConstant;
5049
this.undefined = undefined;
5150
this.sourceSection = sourceSection;
5251
}
@@ -60,11 +59,11 @@ public String getName() {
6059
}
6160

6261
public boolean hasValue() {
63-
return !autoload && !undefined;
62+
return !isAutoload() && !undefined;
6463
}
6564

6665
public Object getValue() {
67-
assert !autoload && !undefined;
66+
assert hasValue();
6867
return value;
6968
}
7069

@@ -84,21 +83,21 @@ public RubyConstant withPrivate(boolean isPrivate) {
8483
if (isPrivate == this.isPrivate) {
8584
return this;
8685
} else {
87-
return new RubyConstant(declaringModule, name, value, isPrivate, autoload, undefined, isDeprecated, sourceSection);
86+
return new RubyConstant(declaringModule, name, value, isPrivate, autoloadConstant, undefined, isDeprecated, sourceSection);
8887
}
8988
}
9089

9190
public RubyConstant withDeprecated() {
9291
if (this.isDeprecated()) {
9392
return this;
9493
} else {
95-
return new RubyConstant(declaringModule, name, value, isPrivate, autoload, undefined, true, sourceSection);
94+
return new RubyConstant(declaringModule, name, value, isPrivate, autoloadConstant, undefined, true, sourceSection);
9695
}
9796
}
9897

9998
public RubyConstant undefined() {
100-
assert autoload;
101-
return new RubyConstant(declaringModule, name, null, isPrivate, false, true, isDeprecated, sourceSection);
99+
assert isAutoload();
100+
return new RubyConstant(declaringModule, name, null, isPrivate, null, true, isDeprecated, sourceSection);
102101
}
103102

104103
@TruffleBoundary
@@ -142,41 +141,11 @@ public boolean isUndefined() {
142141
}
143142

144143
public boolean isAutoload() {
145-
return autoload;
144+
return autoloadConstant != null;
146145
}
147146

148-
public DynamicObject getAutoloadPath() {
149-
assert autoload;
150-
final Object feature = value;
151-
assert RubyGuards.isRubyString(feature);
152-
return (DynamicObject) feature;
153-
}
154-
155-
private ReentrantLock getAutoloadLock() {
156-
synchronized (this) {
157-
if (autoloadLock == null) {
158-
autoloadLock = new ReentrantLock();
159-
}
160-
}
161-
return autoloadLock;
162-
}
163-
164-
@TruffleBoundary
165-
public void startAutoLoad() {
166-
getAutoloadLock().lock();
167-
}
168-
169-
@TruffleBoundary
170-
public void stopAutoLoad() {
171-
getAutoloadLock().unlock();
172-
}
173-
174-
public boolean isAutoloading() {
175-
return autoloadLock != null && autoloadLock.isLocked();
176-
}
177-
178-
public boolean isAutoloadingThread() {
179-
return autoloadLock != null && autoloadLock.isHeldByCurrentThread();
147+
public AutoloadConstant getAutoloadConstant() {
148+
return autoloadConstant;
180149
}
181150

182151
@Override

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -60,17 +60,17 @@ protected Object getConstant(LexicalScope lexicalScope, DynamicObject module, St
6060
protected Object autoloadConstant(LexicalScope lexicalScope, DynamicObject module, String name, RubyConstant autoloadConstant, LookupConstantInterface lookupConstantNode,
6161
@Cached("createPrivate()") CallDispatchHeadNode callRequireNode) {
6262

63-
final DynamicObject feature = autoloadConstant.getAutoloadPath();
63+
final DynamicObject feature = autoloadConstant.getAutoloadConstant().getFeature();
6464

65-
if (autoloadConstant.isAutoloadingThread()) {
65+
if (autoloadConstant.getAutoloadConstant().isAutoloadingThread()) {
6666
// Pretend the constant does not exist while it is autoloading
6767
return doMissingConstant(module, name, getSymbol(name));
6868
}
6969

7070
if (getContext().getOptions().LOG_AUTOLOAD) {
7171
RubyLanguage.LOGGER.info(() -> String.format("%s: autoloading %s with %s",
7272
getContext().fileLine(getContext().getCallStack().getTopMostUserSourceSection()),
73-
autoloadConstant, autoloadConstant.getAutoloadPath()));
73+
autoloadConstant, autoloadConstant.getAutoloadConstant().getAutoloadPath()));
7474
}
7575

7676
final Runnable require = () -> callRequireNode.call(coreLibrary().getMainObject(), "require", feature);
@@ -82,7 +82,7 @@ public Object autoloadConstant(LexicalScope lexicalScope, DynamicObject module,
8282
final DynamicObject autoloadConstantModule = autoloadConstant.getDeclaringModule();
8383
final ModuleFields fields = Layouts.MODULE.getFields(autoloadConstantModule);
8484

85-
autoloadConstant.startAutoLoad();
85+
autoloadConstant.getAutoloadConstant().startAutoLoad();
8686
try {
8787

8888
// We need to notify cached lookup that we are autoloading the constant, as constant
@@ -110,7 +110,7 @@ public Object autoloadConstant(LexicalScope lexicalScope, DynamicObject module,
110110
return executeGetConstant(lexicalScope, module, name, resolvedConstant, lookupConstantNode);
111111

112112
} finally {
113-
autoloadConstant.stopAutoLoad();
113+
autoloadConstant.getAutoloadConstant().stopAutoLoad();
114114
}
115115
}
116116

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

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@
4949
import java.util.ArrayList;
5050
import java.util.Collections;
5151
import java.util.HashMap;
52+
import java.util.LinkedHashMap;
5253
import java.util.List;
5354
import java.util.Map;
5455
import java.util.concurrent.locks.ReentrantLock;
@@ -84,12 +85,13 @@ public void initialize(NativeConfiguration nativeConfiguration, TruffleNFIPlatfo
8485
}
8586

8687
public void addAutoload(RubyConstant autoloadConstant) {
87-
final String basename = basenameWithoutExtension(StringOperations.getString(autoloadConstant.getAutoloadPath()));
88+
final String autoloadPath = autoloadConstant.getAutoloadConstant().getAutoloadPath();
89+
final String basename = basenameWithoutExtension(autoloadPath);
8890

8991
registeredAutoloadsLock.lock();
9092
try {
91-
final Map<String, RubyConstant> constants = ConcurrentOperations.getOrCompute(registeredAutoloads, basename, k -> new HashMap<>());
92-
constants.put(StringOperations.getString(autoloadConstant.getAutoloadPath()), autoloadConstant);
93+
final Map<String, RubyConstant> constants = ConcurrentOperations.getOrCompute(registeredAutoloads, basename, k -> new LinkedHashMap<>());
94+
constants.put(autoloadPath, autoloadConstant);
9395
} finally {
9496
registeredAutoloadsLock.unlock();
9597
}
@@ -111,14 +113,27 @@ public RubyConstant isAutoloadPath(String expandedPath) {
111113
}
112114

113115
for (RubyConstant constant : constants) {
114-
final String expandedAutoloadPath = findFeature(StringOperations.getString(constant.getAutoloadPath()));
116+
final String expandedAutoloadPath = findFeature(constant.getAutoloadConstant().getAutoloadPath());
115117
if (expandedPath.equals(expandedAutoloadPath)) {
116118
return constant;
117119
}
118120
}
119121
return null;
120122
}
121123

124+
public void removeAutoload(RubyConstant constant) {
125+
final String autoloadPath = constant.getAutoloadConstant().getAutoloadPath();
126+
final String basename = basenameWithoutExtension(autoloadPath);
127+
128+
registeredAutoloadsLock.lock();
129+
try {
130+
final Map<String, RubyConstant> constantsMap = registeredAutoloads.get(basename);
131+
constantsMap.remove(autoloadPath, constant);
132+
} finally {
133+
registeredAutoloadsLock.unlock();
134+
}
135+
}
136+
122137
private String basenameWithoutExtension(String path) {
123138
final String basename = new File(path).getName();
124139
int i = basename.lastIndexOf('.');

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ private boolean requireConsideringAutoload(String feature, String expandedPath,
112112
final RubyConstant autoloadConstant = featureLoader.isAutoloadPath(expandedPath);
113113
if (autoloadConstant != null &&
114114
// Do not autoload recursively from the #require call in GetConstantNode
115-
!autoloadConstant.isAutoloading()) {
115+
!autoloadConstant.getAutoloadConstant().isAutoloading()) {
116116
if (getConstantNode == null) {
117117
CompilerDirectives.transferToInterpreterAndInvalidate();
118118
getConstantNode = insert(GetConstantNode.create());
@@ -125,12 +125,16 @@ private boolean requireConsideringAutoload(String feature, String expandedPath,
125125
if (getContext().getOptions().LOG_AUTOLOAD) {
126126
RubyLanguage.LOGGER.info(() -> String.format("%s: requiring %s which is registered as an autoload for %s with %s",
127127
getContext().fileLine(getContext().getCallStack().getTopMostUserSourceSection()),
128-
feature, autoloadConstant, autoloadConstant.getAutoloadPath()));
128+
feature, autoloadConstant, autoloadConstant.getAutoloadConstant().getAutoloadPath()));
129129
}
130130

131131
boolean[] result = new boolean[1];
132132
Runnable require = () -> result[0] = doRequire(feature, expandedPath, pathString);
133-
getConstantNode.autoloadConstant(LexicalScope.IGNORE, autoloadConstant.getDeclaringModule(), autoloadConstant.getName(), autoloadConstant, lookupConstantNode, require);
133+
try {
134+
getConstantNode.autoloadConstant(LexicalScope.IGNORE, autoloadConstant.getDeclaringModule(), autoloadConstant.getName(), autoloadConstant, lookupConstantNode, require);
135+
} finally {
136+
featureLoader.removeAutoload(autoloadConstant);
137+
}
134138
return result[0];
135139
} else {
136140
return doRequire(feature, expandedPath, pathString);

0 commit comments

Comments
 (0)