[GR-8410] Produce reliable LoadSourceEvents.

PullRequest: graal/1075

truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/SourceListenerTest.java

@@ -32,13 +32,16 @@
 import org.junit.Assert;
 import org.junit.Test;
 
+import com.oracle.truffle.api.Truffle;
 import com.oracle.truffle.api.instrumentation.LoadSourceEvent;
 import com.oracle.truffle.api.instrumentation.LoadSourceListener;
 import com.oracle.truffle.api.instrumentation.SourceFilter;
 import com.oracle.truffle.api.instrumentation.SourceSectionFilter;
 import com.oracle.truffle.api.instrumentation.TruffleInstrument;
 import com.oracle.truffle.api.instrumentation.SourceSectionFilter.IndexRange;
 import com.oracle.truffle.api.instrumentation.TruffleInstrument.Registration;
+import com.oracle.truffle.api.nodes.Node;
+import com.oracle.truffle.api.nodes.RootNode;
 import com.oracle.truffle.api.source.SourceSection;
 
 import org.graalvm.polyglot.Instrument;
@@ -117,13 +120,24 @@ private void testLoadSourceImpl(int runTimes) throws IOException {
         assertEvents(impl.allEvents, source4);
     }
 
+    private static void assertEvents(List<com.oracle.truffle.api.source.Source> actualSources) {
+        Assert.assertEquals(0, actualSources.size());
+    }
+
     private void assertEvents(List<com.oracle.truffle.api.source.Source> actualSources, Source... expectedSources) {
         Assert.assertEquals(expectedSources.length, actualSources.size());
         for (int i = 0; i < expectedSources.length; i++) {
             Assert.assertSame("index " + i, getSourceImpl(expectedSources[i]), actualSources.get(i));
         }
     }
 
+    private static void assertEvents(List<com.oracle.truffle.api.source.Source> actualSources, com.oracle.truffle.api.source.Source... expectedSources) {
+        Assert.assertEquals(expectedSources.length, actualSources.size());
+        for (int i = 0; i < expectedSources.length; i++) {
+            Assert.assertSame("index " + i, expectedSources[i], actualSources.get(i));
+        }
+    }
+
     @Registration(id = "testLoadSource1", services = SourceListenerTest.TestLoadSource1.class)
     public static class TestLoadSource1 extends TruffleInstrument {
         List<com.oracle.truffle.api.source.Source> onlyNewEvents = new ArrayList<>();
@@ -247,4 +261,24 @@ protected void onDispose(Env env) {
         }
     }
 
+    @Test
+    public void testLoadSourceNoRootSection() throws Exception {
+        Instrument instrument = engine.getInstruments().get("testLoadSource1");
+        TestLoadSource1 impl = instrument.lookup(TestLoadSource1.class);
+        com.oracle.truffle.api.source.Source source1 = com.oracle.truffle.api.source.Source.newBuilder("line1\nline2").mimeType("mime").name("NoName1").build();
+        com.oracle.truffle.api.source.Source source2 = com.oracle.truffle.api.source.Source.newBuilder("line3\nline4").mimeType("mime").name("NoName2").build();
+        com.oracle.truffle.api.source.Source source3 = com.oracle.truffle.api.source.Source.newBuilder("line5\nline6").mimeType("mime").name("NoName3").build();
+        Node node1 = new SourceSectionFilterTest.SourceSectionNode(source1.createSection(1));
+        RootNode rootA = SourceSectionFilterTest.createRootNode(engine, null, Boolean.FALSE, node1);
+        assertEvents(impl.allEvents);
+        Truffle.getRuntime().createCallTarget(rootA);
+        assertEvents(impl.allEvents, source1);
+
+        Node node2 = new SourceSectionFilterTest.SourceSectionNode(source2.createSection(2));
+        Node node3 = new SourceSectionFilterTest.SourceSectionNode(source3.createSection(2));
+        RootNode rootB = SourceSectionFilterTest.createRootNode(engine, null, Boolean.FALSE, node2, node3);
+        assertEvents(impl.allEvents, source1);
+        Truffle.getRuntime().createCallTarget(rootB);
+        assertEvents(impl.allEvents, source1, source2, source3);
+    }
 }

truffle/src/com.oracle.truffle.api.instrumentation.test/src/com/oracle/truffle/api/instrumentation/test/SourceSectionFilterTest.java

@@ -93,7 +93,7 @@ private static boolean isInstrumentedRoot(SourceSectionFilter filter, Node root,
         }
     }
 
-    private static class SourceSectionNode extends Node implements InstrumentableNode {
+    static class SourceSectionNode extends Node implements InstrumentableNode {
         private final SourceSection sourceSection;
         private final Class<?>[] tags;
 
@@ -130,8 +130,13 @@ private static Node createNode(final SourceSection section, final Class<?>... ta
         return new SourceSectionNode(section, tags);
     }
 
-    private static RootNode createRootNode(final SourceSection section, final Boolean internal) throws Exception {
-        Language language = Engine.create().getLanguages().get(InstrumentationTestLanguage.ID);
+    static RootNode createRootNode(final SourceSection section, final Boolean internal, Node... children) throws Exception {
+        Engine engine = Engine.create();
+        return createRootNode(engine, section, internal, children);
+    }
+
+    static RootNode createRootNode(Engine engine, final SourceSection section, final Boolean internal, Node... children) throws Exception {
+        Language language = engine.getLanguages().get(InstrumentationTestLanguage.ID);
         Field impl = Language.class.getDeclaredField("impl");
         ReflectionUtils.setAccessible(impl, true);
         Object polyglotLanguage = impl.get(language);
@@ -147,6 +152,13 @@ private static RootNode createRootNode(final SourceSection section, final Boolea
 
         return new RootNode(truffleLanguage) {
 
+            @Node.Children Node[] rootChildren = children;
+
+            @Override
+            protected boolean isInstrumentable() {
+                return true;
+            }
+
             @Override
             public SourceSection getSourceSection() {
                 return section;

truffle/src/com.oracle.truffle.api.instrumentation/src/com/oracle/truffle/api/instrumentation/InstrumentationHandler.java

@@ -97,6 +97,7 @@ final class InstrumentationHandler {
     private final Collection<EventBinding.Source<?>> executionBindings = new EventBindingList<>(8);
     private final Collection<EventBinding.Source<?>> sourceSectionBindings = new EventBindingList<>(8);
     private final Collection<EventBinding.Source<?>> sourceBindings = new EventBindingList<>(8);
+    private final FindSourcesVisitor findSourcesVisitor = new FindSourcesVisitor();
     private final Collection<EventBinding<? extends OutputStream>> outputStdBindings = new EventBindingList<>(1);
     private final Collection<EventBinding<? extends OutputStream>> outputErrBindings = new EventBindingList<>(1);
     private final Collection<EventBinding.Allocation<? extends AllocationListener>> allocationBindings = new EventBindingList<>(2);
@@ -132,31 +133,29 @@ void onLoad(RootNode root) {
             return;
         }
         assert root.getLanguageInfo() != null;
-        Source source = null;
+        final Source[] rootSources;
         synchronized (sources) {
             if (!sourceBindings.isEmpty()) {
                 // we'll add to the sourcesList, so it needs to be initialized
                 lazyInitializeSourcesList();
 
                 SourceSection sourceSection = root.getSourceSection();
                 if (sourceSection != null) {
-                    // notify sources
-                    source = sourceSection.getSource();
-                    if (!sources.containsKey(source)) {
-                        sources.put(source, null);
-                        sourcesList.add(source);
-                    } else {
-                        // The source is not new
-                        source = null;
-                    }
+                    findSourcesVisitor.adoptSource(sourceSection.getSource());
                 }
+                visitRoot(root, root, findSourcesVisitor, false);
+                rootSources = findSourcesVisitor.getSources();
+            } else {
+                rootSources = null;
             }
             loadedRoots.add(root);
         }
         // we don't want to invoke foreign code while we are holding a lock to avoid
         // deadlocks.
-        if (source != null) {
-            notifySourceBindingsLoaded(sourceBindings, source);
+        if (rootSources != null) {
+            for (Source src : rootSources) {
+                notifySourceBindingsLoaded(sourceBindings, src);
+            }
         }
 
         // fast path no bindings attached
@@ -166,6 +165,43 @@ void onLoad(RootNode root) {
 
     }
 
+    private class FindSourcesVisitor extends AbstractNodeVisitor {
+
+        private final List<Source> rootSources = new ArrayList<>(5);
+
+        FindSourcesVisitor() {
+        }
+
+        @Override
+        boolean shouldVisit() {
+            return true;
+        }
+
+        @Override
+        protected void visitInstrumentable(Node parentInstrumentable, SourceSection parentSourceSection, Node instrumentableNode, SourceSection sourceSection) {
+            adoptSource(sourceSection.getSource());
+        }
+
+        void adoptSource(Source source) {
+            assert Thread.holdsLock(sources);
+            if (!sources.containsKey(source)) {
+                sources.put(source, null);
+                sourcesList.add(source);
+                rootSources.add(source);
+            }
+        }
+
+        Source[] getSources() {
+            if (rootSources.isEmpty()) {
+                return null;
+            }
+            Source[] sourcesArray = rootSources.toArray(new Source[rootSources.size()]);
+            rootSources.clear();
+            return sourcesArray;
+        }
+
+    }
+
     void onFirstExecution(RootNode root) {
         if (!AccessorInstrumentHandler.nodesAccess().isInstrumentable(root)) {
             return;