Skip to content

Commit 09f09ae

Browse files
eregoneregon
committed
[GR-18163] Fix constant lookup when loading the same file multiple times
PullRequest: truffleruby/2835
2 parents 23b756f + 8dc0f4f commit 09f09ae

File tree

13 files changed

+131
-96
lines changed

13 files changed

+131
-96
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ Bug fixes:
1212
* Fix `Dir.mkdir` error handling for `Pathname` paths (#2397).
1313
* `BasicSocket#*_nonblock(exception: false)` now only return `:wait_readable/:wait_writable` for `EAGAIN`/`EWOULDBLOCK` like MRI (#2400).
1414
* Fix issue with `strspn` used in the `date` C extension compiled as a macro on older glibc and then missing the `__strspn_c1` symbol on newer glibc (#2406).
15+
* Fix constant lookup when loading the same file multiple times (#2408).
1516

1617
Compatibility:
1718

‎src/main/java/org/truffleruby/RubyContext.java

Lines changed: 4 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
*/
1010
package org.truffleruby;
1111

12-
import java.io.File;
1312
import java.io.OutputStream;
1413
import java.io.PrintStream;
1514
import java.io.UnsupportedEncodingException;
@@ -89,7 +88,6 @@
8988

9089
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
9190
import com.oracle.truffle.api.CompilerDirectives.TruffleBoundary;
92-
import com.oracle.truffle.api.TruffleFile;
9391
import com.oracle.truffle.api.TruffleLanguage;
9492
import com.oracle.truffle.api.TruffleLanguage.Env;
9593
import com.oracle.truffle.api.instrumentation.Instrumenter;
@@ -107,8 +105,6 @@ public class RubyContext {
107105

108106
@CompilationFinal public TruffleLogger logger;
109107
@CompilationFinal private Options options;
110-
@CompilationFinal private String rubyHome;
111-
@CompilationFinal private TruffleFile rubyHomeTruffleFile;
112108
@CompilationFinal private boolean hadHome;
113109

114110
private final SafepointManager safepointManager;
@@ -125,7 +121,7 @@ public class RubyContext {
125121
private final CallStackManager callStack;
126122
private final CoreExceptions coreExceptions;
127123
private final EncodingManager encodingManager;
128-
private final MetricsProfiler metricsProfiler = new MetricsProfiler(this);
124+
private final MetricsProfiler metricsProfiler;
129125
private final WeakValueCache<RegexpCacheKey, Regex> regexpCache = new WeakValueCache<>();
130126
private final PreInitializationManager preInitializationManager;
131127
private final NativeConfiguration nativeConfiguration;
@@ -180,6 +176,7 @@ public RubyContext(RubyLanguage language, TruffleLanguage.Env env) {
180176
coreExceptions = new CoreExceptions(this, language);
181177
encodingManager = new EncodingManager(this, language);
182178

179+
metricsProfiler = new MetricsProfiler(language, this);
183180
codeLoader = new CodeLoader(language, this);
184181
featureLoader = new FeatureLoader(this, language);
185182
referenceProcessor = new ReferenceProcessor(this);
@@ -194,9 +191,6 @@ public RubyContext(RubyLanguage language, TruffleLanguage.Env env) {
194191
defaultBacktraceFormatter = BacktraceFormatter.createDefaultFormatter(this, language);
195192
userBacktraceFormatter = new BacktraceFormatter(this, language, BacktraceFormatter.USER_BACKTRACE_FLAGS);
196193

197-
rubyHome = findRubyHome();
198-
rubyHomeTruffleFile = rubyHome == null ? null : env.getInternalTruffleFile(rubyHome);
199-
200194
// Load the core library classes
201195

202196
Metrics.printTime("before-create-core-library");
@@ -263,9 +257,7 @@ public void initialize() {
263257
// Cannot save the file descriptor in this SecureRandom in the image
264258
random = null;
265259
// Do not save image generator paths in the image heap
266-
hadHome = rubyHome != null;
267-
rubyHome = null;
268-
rubyHomeTruffleFile = null;
260+
hadHome = language.getRubyHome() != null;
269261
featureLoader.setWorkingDirectory(null);
270262
} else {
271263
initialized = true;
@@ -283,7 +275,7 @@ protected boolean patch(Env newEnv) {
283275

284276
final Options oldOptions = this.options;
285277
final Options newOptions = createOptions(newEnv, language.options);
286-
final String newHome = findRubyHome();
278+
final String newHome = language.getRubyHome();
287279
if (!compatibleOptions(oldOptions, newOptions, this.hadHome, newHome != null)) {
288280
return false;
289281
}
@@ -294,8 +286,6 @@ protected boolean patch(Env newEnv) {
294286
if (newOptions.WARN_EXPERIMENTAL != oldOptions.WARN_EXPERIMENTAL) {
295287
warningCategoryExperimental.set(newOptions.WARN_EXPERIMENTAL);
296288
}
297-
this.rubyHome = newHome;
298-
this.rubyHomeTruffleFile = newHome == null ? null : newEnv.getInternalTruffleFile(newHome);
299289

300290
// Re-read the value of $TZ as it can be different in the new process
301291
GetTimeZoneNode.invalidateTZ();
@@ -675,14 +665,6 @@ public TruffleLogger getLogger() {
675665
return logger;
676666
}
677667

678-
public String getRubyHome() {
679-
return rubyHome;
680-
}
681-
682-
public TruffleFile getRubyHomeTruffleFile() {
683-
return rubyHomeTruffleFile;
684-
}
685-
686668
public ConsoleHolder getConsoleHolder() {
687669
if (consoleHolder == null) {
688670
synchronized (this) {
@@ -711,50 +693,6 @@ public boolean isFinalizing() {
711693
return finalizing;
712694
}
713695

714-
private String findRubyHome() {
715-
final String home = searchRubyHome();
716-
if (RubyLanguage.LOGGER.isLoggable(Level.CONFIG)) {
717-
RubyLanguage.LOGGER.config("home: " + home);
718-
}
719-
return home;
720-
}
721-
722-
// Returns a canonical path to the home
723-
private String searchRubyHome() {
724-
if (language.options.NO_HOME_PROVIDED) {
725-
RubyLanguage.LOGGER.config("--ruby.no-home-provided set");
726-
return null;
727-
}
728-
729-
final String truffleReported = language.getTruffleLanguageHome();
730-
if (truffleReported != null) {
731-
final File home = new File(truffleReported);
732-
if (isRubyHome(home)) {
733-
RubyLanguage.LOGGER.config(
734-
() -> String.format("Using Truffle-reported home %s as the Ruby home", truffleReported));
735-
return truffleReported;
736-
} else {
737-
RubyLanguage.LOGGER.warning(
738-
String.format(
739-
"Truffle-reported home %s does not look like TruffleRuby's home",
740-
truffleReported));
741-
}
742-
} else {
743-
RubyLanguage.LOGGER.config("Truffle-reported home not set, cannot determine home from it");
744-
}
745-
746-
RubyLanguage.LOGGER.warning(
747-
"could not determine TruffleRuby's home - the standard library will not be available - use --log.level=CONFIG to see details");
748-
return null;
749-
}
750-
751-
private boolean isRubyHome(File path) {
752-
final File lib = new File(path, "lib");
753-
return new File(lib, "truffle").isDirectory() &&
754-
new File(lib, "gems").isDirectory() &&
755-
new File(lib, "patches").isDirectory();
756-
}
757-
758696
public TruffleNFIPlatform getTruffleNFI() {
759697
return truffleNFIPlatform;
760698
}
@@ -796,14 +734,6 @@ public WeakValueCache<RegexpCacheKey, Regex> getRegexpCache() {
796734
return regexpCache;
797735
}
798736

799-
public String getPathRelativeToHome(String path) {
800-
if (path.startsWith(rubyHome) && path.length() > rubyHome.length()) {
801-
return path.substring(rubyHome.length() + 1);
802-
} else {
803-
return path;
804-
}
805-
}
806-
807737
public Object getTopScopeObject() {
808738
return coreLibrary.topScopeObject;
809739
}

‎src/main/java/org/truffleruby/RubyLanguage.java

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,11 @@
1717
import java.util.Optional;
1818
import java.util.concurrent.atomic.AtomicLong;
1919
import java.util.concurrent.locks.ReentrantLock;
20+
import java.util.logging.Level;
2021

2122
import com.oracle.truffle.api.CompilerAsserts;
2223
import com.oracle.truffle.api.CompilerDirectives;
24+
import com.oracle.truffle.api.TruffleFile;
2325
import com.oracle.truffle.api.instrumentation.AllocationReporter;
2426
import com.oracle.truffle.api.instrumentation.Instrumenter;
2527
import com.oracle.truffle.api.object.Shape;
@@ -196,6 +198,8 @@ public final class RubyLanguage extends TruffleLanguage<RubyContext> {
196198
public final ValueWrapperManager.HandleBlockAllocator handleBlockAllocator = new ValueWrapperManager.HandleBlockAllocator();
197199

198200
@CompilationFinal public LanguageOptions options;
201+
@CompilationFinal private String rubyHome;
202+
@CompilationFinal private TruffleFile rubyHomeTruffleFile;
199203

200204
@CompilationFinal private AllocationReporter allocationReporter;
201205
@CompilationFinal public CoverageManager coverageManager;
@@ -366,6 +370,7 @@ public RubyContext createContext(Env env) {
366370
if (this.options == null) { // First context
367371
this.allocationReporter = env.lookup(AllocationReporter.class);
368372
this.options = new LanguageOptions(env, env.getOptions(), singleContext);
373+
setRubyHome(env, findRubyHome());
369374
this.coreLoadPath = buildCoreLoadPath(this.options.CORE_LOAD_PATH);
370375
this.corePath = coreLoadPath + File.separator + "core" + File.separator;
371376
this.coverageManager = new CoverageManager(options, env.lookup(Instrumenter.class));
@@ -393,6 +398,9 @@ protected void initializeContext(RubyContext context) throws Exception {
393398
try {
394399
Metrics.printTime("before-initialize-context");
395400
context.initialize();
401+
if (context.isPreInitializing()) {
402+
setRubyHome(context.getEnv(), null);
403+
}
396404
Metrics.printTime("after-initialize-context");
397405
} catch (Throwable e) {
398406
if (context.getOptions().EXCEPTIONS_PRINT_JAVA || context.getOptions().EXCEPTIONS_PRINT_UNCAUGHT_JAVA) {
@@ -422,6 +430,8 @@ protected boolean patchContext(RubyContext context, Env newEnv) {
422430
return false;
423431
}
424432

433+
setRubyHome(newEnv, findRubyHome());
434+
425435
boolean patched = context.patchContext(newEnv);
426436
Metrics.printTime("after-patch-context");
427437
return patched;
@@ -602,8 +612,70 @@ protected Object getScope(RubyContext context) {
602612
return context.getTopScopeObject();
603613
}
604614

605-
public String getTruffleLanguageHome() {
606-
return getLanguageHome();
615+
public String getRubyHome() {
616+
return rubyHome;
617+
}
618+
619+
public TruffleFile getRubyHomeTruffleFile() {
620+
return rubyHomeTruffleFile;
621+
}
622+
623+
public String getPathRelativeToHome(String path) {
624+
final String home = rubyHome;
625+
if (home != null && path.startsWith(home) && path.length() > home.length()) {
626+
return path.substring(home.length() + 1);
627+
} else {
628+
return path;
629+
}
630+
}
631+
632+
private void setRubyHome(Env env, String home) {
633+
rubyHome = home;
634+
rubyHomeTruffleFile = home == null ? null : env.getInternalTruffleFile(rubyHome);
635+
}
636+
637+
private String findRubyHome() {
638+
final String home = searchRubyHome();
639+
if (RubyLanguage.LOGGER.isLoggable(Level.CONFIG)) {
640+
RubyLanguage.LOGGER.config("home: " + home);
641+
}
642+
return home;
643+
}
644+
645+
// Returns a canonical path to the home
646+
private String searchRubyHome() {
647+
if (options.NO_HOME_PROVIDED) {
648+
RubyLanguage.LOGGER.config("--ruby.no-home-provided set");
649+
return null;
650+
}
651+
652+
final String truffleReported = getLanguageHome();
653+
if (truffleReported != null) {
654+
final File home = new File(truffleReported);
655+
if (isRubyHome(home)) {
656+
RubyLanguage.LOGGER.config(
657+
() -> String.format("Using Truffle-reported home %s as the Ruby home", truffleReported));
658+
return truffleReported;
659+
} else {
660+
RubyLanguage.LOGGER.warning(
661+
String.format(
662+
"Truffle-reported home %s does not look like TruffleRuby's home",
663+
truffleReported));
664+
}
665+
} else {
666+
RubyLanguage.LOGGER.config("Truffle-reported home not set, cannot determine home from it");
667+
}
668+
669+
RubyLanguage.LOGGER.warning(
670+
"could not determine TruffleRuby's home - the standard library will not be available - use --log.level=CONFIG to see details");
671+
return null;
672+
}
673+
674+
private boolean isRubyHome(File path) {
675+
final File lib = new File(path, "lib");
676+
return new File(lib, "truffle").isDirectory() &&
677+
new File(lib, "gems").isDirectory() &&
678+
new File(lib, "patches").isDirectory();
607679
}
608680

609681
@SuppressFBWarnings("IS2_INCONSISTENT_SYNC")

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -565,7 +565,7 @@ private ConcurrentMap<String, Boolean> initializePatching(RubyContext context) {
565565
defineModule(truffleModule, "Patching");
566566
final ConcurrentMap<String, Boolean> patchFiles = new ConcurrentHashMap<>();
567567

568-
final String rubyHome = context.getRubyHome();
568+
final String rubyHome = language.getRubyHome();
569569
if (context.getOptions().PATCHING && rubyHome != null) {
570570
try {
571571
final Path patchesDirectory = Paths.get(rubyHome, "lib", "patches");

‎src/main/java/org/truffleruby/core/rope/RopeNodes.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1316,7 +1316,7 @@ protected boolean fullRopeEqual(Rope a, Rope b,
13161316

13171317
protected boolean canBeCached(Rope a, Rope b) {
13181318
if (getContext().isPreInitializing()) {
1319-
final String home = getContext().getRubyHome();
1319+
final String home = getLanguage().getRubyHome();
13201320
return !RopeOperations.anyChildContains(a, home) && !RopeOperations.anyChildContains(b, home);
13211321
} else {
13221322
return true;

‎src/main/java/org/truffleruby/core/thread/ThreadManager.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -97,9 +97,9 @@ public ThreadManager(RubyContext context, RubyLanguage language) {
9797
}
9898

9999
public void initialize() {
100-
nativeInterrupt = context.getOptions().NATIVE_INTERRUPT && context.getRubyHome() != null;
100+
nativeInterrupt = context.getOptions().NATIVE_INTERRUPT && language.getRubyHome() != null;
101101
if (nativeInterrupt) {
102-
LibRubySignal.loadLibrary(context.getRubyHome());
102+
LibRubySignal.loadLibrary(language.getRubyHome());
103103
LibRubySignal.setupSIGVTALRMEmptySignalHandler();
104104

105105
nativeInterruptTimer = new Timer("Ruby-NativeCallInterrupt-Timer", true);

‎src/main/java/org/truffleruby/debug/MetricsProfiler.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.util.function.Supplier;
1515

1616
import org.truffleruby.RubyContext;
17+
import org.truffleruby.RubyLanguage;
1718
import org.truffleruby.collections.ConcurrentOperations;
1819
import org.truffleruby.shared.options.Profile;
1920

@@ -23,11 +24,13 @@
2324

2425
public class MetricsProfiler {
2526

27+
private final RubyLanguage language;
2628
private final RubyContext context;
2729
/** We need to use the same CallTarget for the same name to appear as one entry to the profiler */
2830
private final Map<String, RootCallTarget> summaryCallTargets = new ConcurrentHashMap<>();
2931

30-
public MetricsProfiler(RubyContext context) {
32+
public MetricsProfiler(RubyLanguage language, RubyContext context) {
33+
this.language = language;
3134
this.context = context;
3235
}
3336

@@ -44,7 +47,7 @@ public <T> T callWithMetrics(String metricKind, String feature, Supplier<T> supp
4447
private <T> RootCallTarget getCallTarget(String metricKind, String feature) {
4548
final String name;
4649
if (context.getOptions().METRICS_PROFILE_REQUIRE == Profile.DETAIL) {
47-
name = "metrics " + metricKind + " " + context.getPathRelativeToHome(feature);
50+
name = "metrics " + metricKind + " " + language.getPathRelativeToHome(feature);
4851
return newCallTarget(name);
4952
} else {
5053
name = "metrics " + metricKind;

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
*/
1010
package org.truffleruby.language;
1111

12+
import com.oracle.truffle.api.CompilerDirectives;
1213
import org.truffleruby.core.module.RubyModule;
1314

1415
import com.oracle.truffle.api.CompilerDirectives.CompilationFinal;
@@ -44,7 +45,20 @@ public RubyModule getLiveModule() {
4445
}
4546

4647
public void unsafeSetLiveModule(RubyModule liveModule) {
47-
this.liveModule = liveModule;
48+
final RubyModule previous = this.liveModule;
49+
if (previous == null) {
50+
this.liveModule = liveModule;
51+
} else if (previous == liveModule) {
52+
// Can be the same module for code like 2.times { module M } (at the toplevel)
53+
} else {
54+
throw CompilerDirectives.shouldNotReachHere(
55+
"Overriding module for static LexicalScope, old=" + moduleToDebugString(previous) +
56+
", new=" + moduleToDebugString(liveModule));
57+
}
58+
}
59+
60+
private static String moduleToDebugString(RubyModule module) {
61+
return module + "@" + Integer.toHexString(module.hashCode());
4862
}
4963

5064
@Override

0 commit comments

Comments
 (0)