Skip to content

Commit 4d134f3

Browse files
committed
[GR-63025] NullPointerException in InternalResourceImageHeapPath.resolve with -H:-IncludeLanguageResources -H:+CopyLanguageResources.
PullRequest: graal/20349
2 parents 6521181 + 2ad1c93 commit 4d134f3

File tree

5 files changed

+158
-63
lines changed

5 files changed

+158
-63
lines changed

truffle/src/com.oracle.truffle.api.test/src/com/oracle/truffle/api/test/polyglot/ContextPreInitializationNativeImageTest.java

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,13 @@
4242

4343
import static org.junit.Assert.assertEquals;
4444
import static org.junit.Assert.assertFalse;
45+
import static org.junit.Assert.assertNotNull;
4546
import static org.junit.Assert.assertTrue;
4647
import static org.junit.Assert.fail;
4748

49+
import com.oracle.truffle.api.InternalResource;
50+
import com.oracle.truffle.api.InternalResource.Id;
51+
import com.oracle.truffle.api.TruffleFile;
4852
import com.oracle.truffle.api.staticobject.DefaultStaticObjectFactory;
4953
import com.oracle.truffle.api.staticobject.DefaultStaticProperty;
5054
import com.oracle.truffle.api.staticobject.StaticProperty;
@@ -66,7 +70,11 @@
6670
import com.oracle.truffle.api.nodes.RootNode;
6771
import com.oracle.truffle.tck.tests.TruffleTestAssumptions;
6872

73+
import java.io.IOException;
6974
import java.lang.reflect.Field;
75+
import java.nio.charset.StandardCharsets;
76+
import java.nio.file.Files;
77+
import java.nio.file.Path;
7078

7179
/**
7280
* Note this test class currently depends on being executed in its own SVM image as it uses the
@@ -129,6 +137,20 @@ public void somShapeAllocatedOnContextPreInit() throws Exception {
129137
somTest.testShapeAllocatedOnContextPreInit();
130138
}
131139

140+
@Test
141+
public void testInternalResourceFilesFromBuildTimeValidInRuntime() throws IOException {
142+
// only supported in AOT
143+
TruffleTestAssumptions.assumeAOT();
144+
TestContext testContext = Language.CONTEXT_REF.get(null);
145+
assertTrue(testContext.home.isAbsolute());
146+
assertTrue(testContext.home.isDirectory());
147+
assertTrue(testContext.lib.isAbsolute());
148+
assertTrue(testContext.lib.isDirectory());
149+
assertTrue(testContext.boot.isAbsolute());
150+
assertTrue(testContext.boot.isRegularFile());
151+
assertEquals("boot library", new String(testContext.boot.readAllBytes(), StandardCharsets.UTF_8));
152+
}
153+
132154
/**
133155
* This test checks that the Static Object Model can be used at image built time for context
134156
* pre-initialization.
@@ -324,6 +346,9 @@ static class TestContext {
324346
final StaticObjectModelTest staticObjectModelTest;
325347
boolean patched;
326348
int threadLocalActions;
349+
TruffleFile home;
350+
TruffleFile lib;
351+
TruffleFile boot;
327352

328353
TestContext(Env env, Language language) {
329354
this.env = env;
@@ -332,6 +357,24 @@ static class TestContext {
332357

333358
}
334359

360+
@Id(value = LanguageHome.ID, componentId = LANGUAGE, optional = true)
361+
static final class LanguageHome implements InternalResource {
362+
static final String ID = "home";
363+
364+
@Override
365+
public void unpackFiles(Env env, Path targetDirectory) throws IOException {
366+
Path lib = targetDirectory.resolve("lib");
367+
Files.createDirectories(lib);
368+
Path coreLib = lib.resolve("boot.txt");
369+
Files.writeString(coreLib, "boot library");
370+
}
371+
372+
@Override
373+
public String versionHash(Env env) {
374+
return "42";
375+
}
376+
}
377+
335378
@TruffleLanguage.Registration(id = LANGUAGE, name = LANGUAGE, version = "1.0", contextPolicy = TruffleLanguage.ContextPolicy.SHARED)
336379
public static final class Language extends TruffleLanguage<TestContext> {
337380

@@ -395,6 +438,20 @@ public Object execute(VirtualFrame frame) {
395438
// No need to call `testShapeAllocatedOnContextPreInit()` here.
396439
// During context pre-init, it is equivalent to `testObjectAllocatedOnContextPreInit()`.
397440
context.staticObjectModelTest.testObjectAllocatedOnContextPreInit();
441+
442+
/*
443+
* Query the internal resource in the context pre-initialization time and keep files
444+
* referring to lib folder bootstrap library file.
445+
*/
446+
TruffleFile homeRoot = context.env.getInternalResource(LanguageHome.ID);
447+
assertNotNull(homeRoot);
448+
TruffleFile libFolder = homeRoot.resolve("lib");
449+
assertTrue(libFolder.isDirectory());
450+
TruffleFile bootFile = libFolder.resolve("boot.txt");
451+
assertTrue(bootFile.isRegularFile());
452+
context.home = homeRoot;
453+
context.lib = libFolder;
454+
context.boot = bootFile;
398455
}
399456

400457
@Override

truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/InternalResourceCache.java

Lines changed: 58 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,7 @@ final class InternalResourceCache {
104104
private final String id;
105105
private final String resourceId;
106106
private final Supplier<InternalResource> resourceFactory;
107+
private boolean requiresEagerUnpack;
107108

108109
/**
109110
* This field is reset to {@code null} by the {@code TruffleBaseFeature} before writing the
@@ -145,6 +146,9 @@ Path getPath(PolyglotEngineImpl polyglotEngine) throws IOException {
145146
}
146147
}
147148
}
149+
if (polyglotEngine.inEnginePreInitialization) {
150+
requiresEagerUnpack = true;
151+
}
148152
return result;
149153
} else {
150154
throw new IllegalArgumentException("Internal resources are restricted. To enable them, use '-H:+CopyLanguageResources' during the native image build.");
@@ -187,6 +191,14 @@ void clearCache() {
187191
path = null;
188192
}
189193

194+
/**
195+
* Returns {@code true} if the resource requires eager installation during pre-initialized
196+
* engine patching.
197+
*/
198+
boolean requiresEagerUnpack() {
199+
return requiresEagerUnpack;
200+
}
201+
190202
/**
191203
* Installs truffleattach library. Used reflectively by
192204
* {@code com.oracle.truffle.runtime.JDKSupport}. The {@code JDKSupport} is initialized before
@@ -326,13 +338,12 @@ static void resetNativeImageState() {
326338
* {@code TruffleBaseFeature#afterImageWrite}.
327339
*/
328340
static boolean copyResourcesForNativeImage(Path target, String... components) throws IOException {
329-
boolean result = false;
330-
Collection<LanguageCache> languages;
331-
Collection<InstrumentCache> instruments;
332-
if (components.length == 0) {
333-
languages = LanguageCache.languages().values();
334-
instruments = InstrumentCache.load();
335-
} else {
341+
boolean[] result = {false};
342+
Set<String> componentFilter;
343+
if (components.length != 0) {
344+
componentFilter = new HashSet<>();
345+
// Always install engine resources
346+
componentFilter.add(PolyglotEngineImpl.ENGINE_ID);
336347
Set<String> requiredComponentIds = new HashSet<>();
337348
Collections.addAll(requiredComponentIds, components);
338349
Set<String> requiredLanguageIds = new HashSet<>(LanguageCache.languages().keySet());
@@ -352,26 +363,20 @@ static boolean copyResourcesForNativeImage(Path target, String... components) th
352363
for (String requiredLanguageId : requiredLanguageIds) {
353364
requiredLanguages.addAll(LanguageCache.computeTransitiveLanguageDependencies(requiredLanguageId));
354365
}
355-
languages = requiredLanguages;
356-
Set<InstrumentCache> requiredInstruments = new HashSet<>(InstrumentCache.internalInstruments());
357-
InstrumentCache.load().stream().filter((ic) -> requiredInstrumentIds.contains(ic.getId())).forEach(requiredInstruments::add);
358-
instruments = requiredInstruments;
359-
}
360-
for (LanguageCache language : languages) {
361-
for (InternalResourceCache cache : language.getResources()) {
362-
result |= cache.copyResourcesForNativeImage(target);
363-
}
366+
requiredLanguages.stream().map(LanguageCache::getId).forEach(componentFilter::add);
367+
InstrumentCache.internalInstruments().stream().map(InstrumentCache::getId).forEach(componentFilter::add);
368+
componentFilter.addAll(requiredInstrumentIds);
369+
} else {
370+
componentFilter = null;
364371
}
365-
for (InstrumentCache instrument : instruments) {
366-
for (InternalResourceCache cache : instrument.getResources()) {
367-
result |= cache.copyResourcesForNativeImage(target);
372+
walkAllResources((componentId, resources) -> {
373+
if (componentFilter == null || componentFilter.contains(componentId)) {
374+
for (InternalResourceCache cache : resources) {
375+
result[0] |= cache.copyResourcesForNativeImage(target);
376+
}
368377
}
369-
}
370-
// Always install engine resources
371-
for (InternalResourceCache cache : getEngineResources()) {
372-
result |= cache.copyResourcesForNativeImage(target);
373-
}
374-
return result;
378+
});
379+
return result[0];
375380
}
376381

377382
private boolean copyResourcesForNativeImage(Path target) throws IOException {
@@ -390,23 +395,12 @@ private boolean copyResourcesForNativeImage(Path target) throws IOException {
390395
}
391396

392397
@SuppressWarnings("unused")
393-
static void includeResourcesForNativeImage(Path tempDir, BiConsumer<Module, Pair<String, byte[]>> resourceLocationConsumer) throws IOException, NoSuchAlgorithmException {
394-
Collection<LanguageCache> languages = LanguageCache.languages().values();
395-
Collection<InstrumentCache> instruments = InstrumentCache.load();
396-
for (LanguageCache language : languages) {
397-
for (InternalResourceCache cache : language.getResources()) {
398+
static void includeResourcesForNativeImage(Path tempDir, BiConsumer<Module, Pair<String, byte[]>> resourceLocationConsumer) throws Exception {
399+
walkAllResources((componentId, resources) -> {
400+
for (InternalResourceCache cache : resources) {
398401
cache.includeResourcesForNativeImageImpl(tempDir, resourceLocationConsumer);
399402
}
400-
}
401-
for (InstrumentCache instrument : instruments) {
402-
for (InternalResourceCache cache : instrument.getResources()) {
403-
cache.includeResourcesForNativeImageImpl(tempDir, resourceLocationConsumer);
404-
}
405-
}
406-
// Always install engine resources
407-
for (InternalResourceCache cache : getEngineResources()) {
408-
cache.includeResourcesForNativeImageImpl(tempDir, resourceLocationConsumer);
409-
}
403+
});
410404
useExternalDirectoryInNativeImage = false;
411405
}
412406

@@ -451,6 +445,30 @@ private void includeResourcesForNativeImageImpl(Path tempDir, BiConsumer<Module,
451445
resourceLocationConsumer.accept(resource.getClass().getModule(), Pair.create(getResourceName(aggregatedFileListResource), fileList.toString().getBytes()));
452446
}
453447

448+
@FunctionalInterface
449+
interface ResourcesVisitor<T extends Throwable> {
450+
void visit(String componentId, Collection<InternalResourceCache> resources) throws T;
451+
}
452+
453+
static <T extends Throwable> void walkAllResources(ResourcesVisitor<T> consumer) throws T {
454+
for (LanguageCache language : LanguageCache.languages().values()) {
455+
Collection<InternalResourceCache> resources = language.getResources();
456+
if (!resources.isEmpty()) {
457+
consumer.visit(language.getId(), language.getResources());
458+
}
459+
}
460+
for (InstrumentCache instrument : InstrumentCache.load()) {
461+
Collection<InternalResourceCache> resources = instrument.getResources();
462+
if (!resources.isEmpty()) {
463+
consumer.visit(instrument.getId(), resources);
464+
}
465+
}
466+
Collection<InternalResourceCache> engineResources = InternalResourceCache.getEngineResources();
467+
if (!engineResources.isEmpty()) {
468+
consumer.visit(PolyglotEngineImpl.ENGINE_ID, engineResources);
469+
}
470+
}
471+
454472
static Collection<String> getEngineResourceIds() {
455473
Map<String, Supplier<InternalResourceCache>> engineResources = loadOptionalInternalResources(EngineAccessor.locatorOrDefaultLoaders()).get(PolyglotEngineImpl.ENGINE_ID);
456474
return engineResources != null ? engineResources.keySet() : List.of();

truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/InternalResourceRoots.java

Lines changed: 33 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
*/
4141
package com.oracle.truffle.polyglot;
4242

43+
import java.io.IOException;
4344
import java.nio.file.InvalidPathException;
4445
import java.nio.file.Path;
4546
import java.nio.file.Paths;
@@ -107,8 +108,34 @@ static InternalResourceRoots getInstance() {
107108
return instance;
108109
}
109110

110-
void patch() {
111+
boolean patch(PolyglotEngineImpl engine) {
111112
ensureInitialized();
113+
/*
114+
* Unpack all resources that are included in the native-image binary and were queried during
115+
* context pre-initialization.
116+
*/
117+
boolean[] result = {true};
118+
InternalResourceCache.walkAllResources((componentId, resources) -> {
119+
for (InternalResourceCache resource : resources) {
120+
if (resource.requiresEagerUnpack()) {
121+
if (InternalResourceCache.usesInternalResources()) {
122+
try {
123+
resource.getPath(engine);
124+
} catch (IOException ioe) {
125+
throw new IllegalStateException(ioe);
126+
}
127+
} else {
128+
/*
129+
* Internal resources were utilized during the image build but are disabled
130+
* at runtime. Return false to indicate that the pre-initialized engine
131+
* should not be used.
132+
*/
133+
result[0] = false;
134+
}
135+
}
136+
}
137+
});
138+
return result[0];
112139
}
113140

114141
private synchronized void ensureInitialized() {
@@ -195,22 +222,9 @@ private static void setTestCacheRoot(Path newRoot, boolean nativeImageRuntime) {
195222
*/
196223
private static Set<Root> computeRoots(Pair<Path, Root.Kind> defaultRoot) {
197224
Map<Pair<Path, Root.Kind>, List<InternalResourceCache>> collector = new HashMap<>();
198-
for (LanguageCache language : LanguageCache.languages().values()) {
199-
Collection<InternalResourceCache> resources = language.getResources();
200-
if (!resources.isEmpty()) {
201-
collectRoots(language.getId(), defaultRoot.getLeft(), defaultRoot.getRight(), resources, collector);
202-
}
203-
}
204-
for (InstrumentCache instrument : InstrumentCache.load()) {
205-
Collection<InternalResourceCache> resources = instrument.getResources();
206-
if (!resources.isEmpty()) {
207-
collectRoots(instrument.getId(), defaultRoot.getLeft(), defaultRoot.getRight(), resources, collector);
208-
}
209-
}
210-
Collection<InternalResourceCache> engineResources = InternalResourceCache.getEngineResources();
211-
if (!engineResources.isEmpty()) {
212-
collectRoots(PolyglotEngineImpl.ENGINE_ID, defaultRoot.getLeft(), defaultRoot.getRight(), engineResources, collector);
213-
}
225+
InternalResourceCache.walkAllResources((componentId, resources) -> {
226+
collectRoots(componentId, defaultRoot.getLeft(), defaultRoot.getRight(), resources, collector);
227+
});
214228
// Build a set of immutable Roots.
215229
Set<Root> result = new HashSet<>();
216230
for (var entry : collector.entrySet()) {
@@ -246,7 +260,7 @@ private static Pair<Path, Root.Kind> findDefaultRoot() {
246260
}
247261

248262
private static void collectRoots(String componentId, Path componentRoot, Root.Kind componentKind, Collection<InternalResourceCache> resources,
249-
Map<Pair<Path, Root.Kind>, List<InternalResourceCache>> collector) {
263+
Map<Pair<Path, Root.Kind>, List<InternalResourceCache>> resourcesCollector) {
250264
Path useRoot = componentRoot;
251265
Root.Kind useKind = componentKind;
252266
String overriddenRoot = System.getProperty(overriddenComponentRootProperty(componentId));
@@ -262,7 +276,7 @@ private static void collectRoots(String componentId, Path componentRoot, Root.Ki
262276
resourceRoot = Path.of(overriddenRoot).toAbsolutePath();
263277
resourceKind = Root.Kind.RESOURCE;
264278
}
265-
collector.computeIfAbsent(Pair.create(resourceRoot, resourceKind), (k) -> new ArrayList<>()).add(resource);
279+
resourcesCollector.computeIfAbsent(Pair.create(resourceRoot, resourceKind), (k) -> new ArrayList<>()).add(resource);
266280
}
267281
}
268282

truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotEngineImpl.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -692,7 +692,7 @@ static OptionDescriptors createEngineOptionDescriptors() {
692692
return LANGUAGE.createOptionDescriptorsUnion(engineOptionDescriptors, compilerOptionDescriptors);
693693
}
694694

695-
void patch(SandboxPolicy newSandboxPolicy,
695+
boolean patch(SandboxPolicy newSandboxPolicy,
696696
DispatchOutputStream newOut,
697697
DispatchOutputStream newErr,
698698
InputStream newIn,
@@ -713,7 +713,9 @@ void patch(SandboxPolicy newSandboxPolicy,
713713
this.logHandler = newLogHandler;
714714
this.engineOptionValues = engineOptions;
715715
this.logLevels = newLogConfig.logLevels;
716-
this.internalResourceRoots.patch();
716+
if (!this.internalResourceRoots.patch(this)) {
717+
return false;
718+
}
717719

718720
if (PreInitContextHostLanguage.isInstance(hostLanguage)) {
719721
this.hostLanguage = createLanguage(LanguageCache.createHostLanguageCache(newHostLanguage), HOST_LANGUAGE_INDEX, null);
@@ -764,6 +766,7 @@ void patch(SandboxPolicy newSandboxPolicy,
764766
}
765767
validateSandbox();
766768
printDeprecatedOptionsWarning(deprecatedDescriptors);
769+
return true;
767770
}
768771

769772
static LogHandler createLogHandler(LogConfig logConfig, DispatchOutputStream errDispatchOutputStream, SandboxPolicy sandboxPolicy) {

truffle/src/com.oracle.truffle.polyglot/src/com/oracle/truffle/polyglot/PolyglotImpl.java

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -324,7 +324,7 @@ public Engine buildEngine(String[] permittedLanguages, SandboxPolicy sandboxPoli
324324

325325
if (impl != null) {
326326
assert hostLanguage.getClass() == impl.getHostLanguageSPI().getClass() || PreInitContextHostLanguage.isInstance(impl.hostLanguage);
327-
impl.patch(sandboxPolicy, dispatchOut,
327+
boolean patchSuccess = impl.patch(sandboxPolicy, dispatchOut,
328328
dispatchErr,
329329
resolvedIn,
330330
engineOptions,
@@ -336,7 +336,10 @@ public Engine buildEngine(String[] permittedLanguages, SandboxPolicy sandboxPoli
336336
useHandler,
337337
(TruffleLanguage<?>) hostLanguage,
338338
usePolyglotHostService);
339-
339+
if (!patchSuccess) {
340+
// Engine patching failed create a new engine
341+
impl = null;
342+
}
340343
}
341344
if (impl == null) {
342345
impl = new PolyglotEngineImpl(this, sandboxPolicy,

0 commit comments

Comments
 (0)