Merge pull request #3144 from apache/delivery

synchronize delivery and branch release125

groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/compiler/CompilationUnit.java

@@ -164,6 +164,7 @@ public final class CompilationUnit extends org.codehaus.groovy.control.Compilati
                 Task<CompilationController> task = new Task<CompilationController>() {
                     @Override
                     public void run(CompilationController controller) throws Exception {
+                        controller.toPhase(JavaSource.Phase.ELEMENTS_RESOLVED);
                         Elements elements = controller.getElements();
                         TypeElement typeElement = ElementSearch.getClass(elements, name);
                         if (typeElement != null) {

groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/completion/MethodCompletion.java

@@ -158,8 +158,8 @@ public class MethodCompletion extends BaseCompletion {
             try {
                 javaSource.runUserActionTask(new Task<CompilationController>() {
                     @Override
-                    public void run(CompilationController info) {
-
+                    public void run(CompilationController info) throws IOException {
+                        info.toPhase(JavaSource.Phase.ELEMENTS_RESOLVED);
                         List<Element> typelist = new ArrayList<>();
                         for (String importName : getAllImports()) {
                             typelist.addAll(getElementListFor(info.getElements(), importName));

groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/completion/TypesCompletion.java

@@ -345,9 +345,9 @@ public class TypesCompletion extends BaseCompletion {
             try {
                 javaSource.runUserActionTask(new Task<CompilationController>() {
                     @Override
-                    public void run(CompilationController info) {
+                    public void run(CompilationController info) throws IOException {
                         Elements elements = info.getElements();
-
+                        info.toPhase(JavaSource.Phase.ELEMENTS_RESOLVED);
                         addPackageElements(elements.getPackageElement(pkg));
                         addTypeElements(elements.getTypeElement(pkg));
                     }

groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/completion/provider/JavaElementHandler.java

@@ -215,7 +215,7 @@ public final class JavaElementHandler {
                     return false;
                 }
             };
-
+            info.toPhase(JavaSource.Phase.ELEMENTS_RESOLVED);
             TypeElement te = elements.getTypeElement(className);
             if (te != null) {
                 for (ExecutableElement element : ElementFilter.methodsIn(te.getEnclosedElements())) {
@@ -383,7 +383,7 @@ public final class JavaElementHandler {
                         return false;
                     }
                 };
-
+                info.toPhase(JavaSource.Phase.ELEMENTS_RESOLVED);
                 TypeElement te = elements.getTypeElement(className);
                 if (te != null) {
                     for (VariableElement element : ElementFilter.fieldsIn(te.getEnclosedElements())) {

groovy/groovy.editor/src/org/netbeans/modules/groovy/editor/java/JavaElementHandle.java

@@ -213,6 +213,7 @@ public final class JavaElementHandle implements ElementHandle {
             @Override
             public void run(CompilationController parameter) throws Exception {
                 try {
+                    parameter.toPhase(JavaSource.Phase.ELEMENTS_RESOLVED);
                     result = resolver.apply(parameter, toElement(parameter));
                 } catch (Exception ex) {
                     thrown = ex;

ide/projectapi/src/org/netbeans/modules/projectapi/LazyLookup.java

+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.projectapi;
+
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.Executor;
+import java.util.logging.Level;
+import org.openide.filesystems.FileObject;
+import org.openide.util.Exceptions;
+import org.openide.util.Lookup;
+import org.openide.util.lookup.Lookups;
+import org.openide.util.lookup.ProxyLookup;
+
+final class LazyLookup extends ProxyLookup {
+    private final Map<String, Object> attrs;
+    private final Lookup lkp;
+    private Collection<String> serviceNames;
+    final Thread[] LOCK = {null};
+
+    public LazyLookup(Map<String, Object> attrs, Lookup lkp) {
+        this.attrs = attrs;
+        this.lkp = lkp;
+        this.serviceNames = Arrays.asList(((String) attrs.get("service")).split(",")); // NOI18N
+    }
+
+    @Override
+    protected void beforeLookup(Template<?> template) {
+        class NotifyLater implements Executor {
+            private List<Runnable> pending = new ArrayList<>();
+            @Override
+            public void execute(Runnable command) {
+                pending.add(command);
+            }
+            
+            public void deliverPending() {
+                List<Runnable> notify = pending;
+                pending = null;
+                for (Runnable r : notify) {
+                    r.run();
+                }
+            }
+        }
+        LazyLookupProviders.safeToLoad(this.lkp);
+        Class<?> service = template.getType();
+        NotifyLater later = null;
+        synchronized (LOCK) {
+            for (;;) {
+                if (serviceNames == null || !serviceNames.contains(service.getName())) {
+                    return;
+                }
+                if (LOCK[0] == null) {
+                    break;
+                }
+                if (LOCK[0] == Thread.currentThread()) {
+                    return;
+                }
+                try {
+                    LOCK.wait();
+                } catch (InterruptedException ex) {
+                    LazyLookupProviders.LOG.log(Level.INFO, null, ex);
+                }
+            }
+            LOCK[0] = Thread.currentThread();
+            LOCK.notifyAll();
+        }
+        try {
+            Object instance = LazyLookupProviders.loadPSPInstance((String) attrs.get("class"), (String) attrs.get("method"), lkp); // NOI18N
+            if (!service.isInstance(instance)) {
+                // JRE #6456938: Class.cast currently throws an exception without details.
+                throw new ClassCastException("Instance of " + instance.getClass() + " unassignable to " + service);
+            }
+            setLookups(later = new NotifyLater(), Lookups.singleton(instance));
+            synchronized (LOCK) {
+                serviceNames = null;
+                LOCK.notifyAll();
+            }
+        } catch (Exception x) {
+            Exceptions.attachMessage(x, "while loading from " + attrs);
+            Exceptions.printStackTrace(x);
+        } finally {
+            synchronized (LOCK) {
+                LOCK[0] = null;
+                LOCK.notifyAll();
+            }
+            if (later != null) {
+                later.deliverPending();
+            }
+        }
+    }
+
+    boolean isInitializing() {
+        return LOCK[0] != null;
+    }
+    
+    @Override
+    @SuppressWarnings(value = "element-type-mismatch")
+    public String toString() {
+        return "LazyLookupProviders.LookupProvider[service=" + attrs.get("service") + ", class=" + attrs.get("class") + ", orig=" + attrs.get(FileObject.class) + "]";
+    }
+
+}

ide/projectapi/src/org/netbeans/modules/projectapi/LazyLookupProviders.java

@@ -23,7 +23,6 @@ import java.lang.reflect.Constructor;
 import java.lang.reflect.Member;
 import java.lang.reflect.Method;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
@@ -37,13 +36,9 @@ import org.netbeans.api.project.Project;
 import org.netbeans.spi.project.LookupMerger;
 import org.netbeans.spi.project.LookupProvider;
 import org.netbeans.spi.project.ProjectServiceProvider;
-import org.openide.filesystems.FileObject;
 import org.openide.util.ChangeSupport;
 import org.openide.util.Exceptions;
 import org.openide.util.Lookup;
-import org.openide.util.Lookup.Template;
-import org.openide.util.lookup.Lookups;
-import org.openide.util.lookup.ProxyLookup;
 
 /**
  * Factory methods for lazy {@link LookupProvider} registration.
@@ -52,7 +47,7 @@ public class LazyLookupProviders {
 
     private LazyLookupProviders() {}
 
-    private static final Logger LOG = Logger.getLogger(LazyLookupProviders.class.getName());
+    static final Logger LOG = Logger.getLogger(LazyLookupProviders.class.getName());
     private static final Map<Lookup,ThreadLocal<Member>> INSIDE_LOAD = new WeakHashMap<Lookup,ThreadLocal<Member>>();
     private static final Collection<Member> WARNED = Collections.synchronizedSet(new HashSet<Member>());
 
@@ -62,97 +57,39 @@ public class LazyLookupProviders {
     public static LookupProvider forProjectServiceProvider(final Map<String,Object> attrs) throws ClassNotFoundException {
         class Prov implements LookupProvider {
             @Override
-            public Lookup createAdditionalLookup(final Lookup lkp) {
-                final Lookup result =  new ProxyLookup() {
-                    Collection<String> serviceNames = Arrays.asList(((String) attrs.get("service")).split(",")); // NOI18N
-                    final Thread[] LOCK = { null };
-                    @Override protected void beforeLookup(Template<?> template) {
-                        safeToLoad();
-                        Class<?> service = template.getType();
-                        synchronized (LOCK) {
-                            for (;;) {
-                                if (serviceNames == null || !serviceNames.contains(service.getName())) {
-                                    return;
-                                }
-                                if (LOCK[0] == null) {
-                                    break;
-                                }
-                                if (LOCK[0] == Thread.currentThread()) {
-                                    return;
-                                }
-                                try {
-                                    LOCK.wait();
-                                } catch (InterruptedException ex) {
-                                    LOG.log(Level.INFO, null, ex);
-                                }
-                            }
-                            LOCK[0] = Thread.currentThread();
-                        }
-                        try {
-                            Object instance = loadPSPInstance((String) attrs.get("class"), (String) attrs.get("method"), lkp); // NOI18N
-                            if (!service.isInstance(instance)) {
-                                // JRE #6456938: Class.cast currently throws an exception without details.
-                                throw new ClassCastException("Instance of " + instance.getClass() + " unassignable to " + service);
-                            }
-                            setLookups(Lookups.singleton(instance));
-                            synchronized (LOCK) {
-                                serviceNames = null;
-                            }
-                        } catch (Exception x) {
-                            Exceptions.attachMessage(x, "while loading from " + attrs);
-                            Exceptions.printStackTrace(x);
-                        } finally {
-                            synchronized (LOCK) {
-                                LOCK[0] = null;
-                                LOCK.notifyAll();
-                            }
-                        }
-                    }
-                    private void safeToLoad() {
-                        ThreadLocal<Member> memberRef;
-                        synchronized (INSIDE_LOAD) {
-                            memberRef = INSIDE_LOAD.get(lkp);
-                        }
-                        if (memberRef == null) {
-                            return;
-                        }
-                        Member member = memberRef.get();
-                        if (member != null && WARNED.add(member)) {
-                            LOG.log(Level.WARNING, null, new IllegalStateException("may not call Project.getLookup().lookup(...) inside " + member.getName() + " registered under @ProjectServiceProvider"));
-                        }
-                    }
-
-                    @Override
-                    public String toString() {
-                        return Prov.this.toString();
-                    }
-                };
+            public Lookup createAdditionalLookup(Lookup lkp) {
+                LazyLookup result = new LazyLookup(attrs, lkp);
                 if (LOG.isLoggable(Level.FINE)) {
                     LOG.log(
-                        Level.FINE,
-                        "Additional lookup created: {0} service class: {1} for base lookup: {2}",   //NOI18N
-                        new Object[]{
+                            Level.FINE,
+                            "Additional lookup created: {0} service class: {1} for base lookup: {2}", //NOI18N
+                            new Object[]{
                             System.identityHashCode(result),
-                            attrs.get("class"),
-                            System.identityHashCode(lkp)
-                        });
+                                attrs.get("class"),
+                                System.identityHashCode(lkp)
+                            });
                 }
                 return result;
             }
-            
-            @Override
-            @SuppressWarnings("element-type-mismatch")
-            public String toString() {
-                return "LazyLookupProviders.LookupProvider[service=" + 
-                    attrs.get("service") + 
-                    ", class=" + attrs.get("class") + 
-                    ", orig=" + attrs.get(FileObject.class) + 
-                    "]";
-            }
-        };
+        }
         return new Prov();
     }
-    private static Object loadPSPInstance(String implName, String methodName, Lookup lkp) throws Exception {
+
+    static void safeToLoad(Lookup lkp) {
+        ThreadLocal<Member> memberRef;
+        synchronized (LazyLookupProviders.INSIDE_LOAD) {
+            memberRef = LazyLookupProviders.INSIDE_LOAD.get(lkp);
+        }
+        if (memberRef == null) {
+            return;
+        }
+        Member member = memberRef.get();
+        if (member != null && LazyLookupProviders.WARNED.add(member)) {
+            LazyLookupProviders.LOG.log(Level.WARNING, null, new IllegalStateException("may not call Project.getLookup().lookup(...) inside " + member.getName() + " registered under @ProjectServiceProvider"));
+        }
+    }
+    
+    static Object loadPSPInstance(String implName, String methodName, Lookup lkp) throws Exception {
         ClassLoader loader = Lookup.getDefault().lookup(ClassLoader.class);
         if (loader == null) {
             loader = Thread.currentThread().getContextClassLoader();

ide/projectapi/src/org/netbeans/spi/project/support/DelegatingLookupImpl.java

@@ -27,6 +27,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.Executor;
 import java.util.logging.Level;
 import java.util.logging.Logger;
 import javax.swing.event.ChangeEvent;
@@ -47,7 +48,8 @@ class DelegatingLookupImpl extends ProxyLookup implements LookupListener, Change
 
     private final Lookup baseLookup;
     private final String pathDescriptor;
-    private final UnmergedLookup unmergedLookup = new UnmergedLookup();
+    private final Controller unmergedController;
+    private final ProxyLookup unmergedLookup;
     private final Map<LookupMerger<?>,Object> mergerResults = new HashMap<LookupMerger<?>,Object>();
     private final Lookup.Result<LookupProvider> providerResult;
     private final LookupListener providerListener;
@@ -63,6 +65,8 @@ class DelegatingLookupImpl extends ProxyLookup implements LookupListener, Change
     @SuppressWarnings("LeakingThisInConstructor")
     DelegatingLookupImpl(Lookup base, Lookup providerLookup, String pathDescriptor) {
         assert base != null;
+        this.unmergedController = new ProxyLookup.Controller();
+        this.unmergedLookup = new ProxyLookup(this.unmergedController);
         baseLookup = base;
         this.pathDescriptor = pathDescriptor;
         providerResult = providerLookup.lookupResult(LookupProvider.class);
@@ -96,6 +100,24 @@ class DelegatingLookupImpl extends ProxyLookup implements LookupListener, Change
     }
 
     private void doDelegate() {
+        class NotifyLater implements Executor {
+            List<Runnable> pending = new ArrayList<>();
+
+            @Override
+            public void execute(Runnable command) {
+                pending.add(command);
+            }
+
+            public void notifyCollectedEvents() {
+                List<Runnable> tmp = pending;
+                pending = null;
+                for (Runnable r : tmp) {
+                    r.run();
+                }
+            }
+        }
+        NotifyLater notifyLater = new NotifyLater();
+
         synchronized (results) {
             for (Lookup.Result<?> r : results) {
                 r.removeLookupListener(this);
@@ -125,7 +147,7 @@ class DelegatingLookupImpl extends ProxyLookup implements LookupListener, Change
             old = new ArrayList<LookupProvider>(providers);
             currentLookups = newLookups;
             newLookups.add(baseLookup);
-            unmergedLookup._setLookups(newLookups.toArray(new Lookup[newLookups.size()]));
+            unmergedController.setLookups(notifyLater, newLookups.toArray(new Lookup[newLookups.size()]));
             List<Class<?>> filteredClasses = new ArrayList<Class<?>>();
             List<Object> mergedInstances = new ArrayList<Object>();
             LookupListener l = listenerRef != null ? listenerRef.get() : null;
@@ -165,14 +187,13 @@ class DelegatingLookupImpl extends ProxyLookup implements LookupListener, Change
             }
             Lookup filtered = Lookups.exclude(unmergedLookup, filteredClasses.toArray(new Class<?>[filteredClasses.size()]));
             Lookup fixed = Lookups.fixed(mergedInstances.toArray(new Object[mergedInstances.size()]));
-            setLookups(fixed, filtered);
+            setLookups(notifyLater, fixed, filtered);
         }
+        notifyLater.notifyCollectedEvents();
     }
 
-    private static class UnmergedLookup extends ProxyLookup {
-        void _setLookups(Lookup... lookups) {
-            setLookups(lookups);
-        }
+    final boolean holdsLock() {
+        return Thread.holdsLock(results);
     }
 
     //just for assertion evaluation.

ide/projectapi/test/unit/src/org/netbeans/modules/projectapi/LazyLookupTest.java

+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.netbeans.modules.projectapi;
+
+import java.util.HashMap;
+import java.util.Map;
+import org.junit.Test;
+import static org.junit.Assert.*;
+import org.openide.util.Lookup;
+import org.openide.util.LookupEvent;
+import org.openide.util.LookupListener;
+
+public class LazyLookupTest {
+    
+    public LazyLookupTest() {
+    }
+    
+    @Test
+    public void testChangesAreNotifiedWithoutHoldingALock() {
+        
+        Map<String,Object> data = new HashMap<>();
+        data.put("service", String.class.getName());
+        data.put("class", LazyLookupTest.class.getName());
+        data.put("method", "testFactory");
+        LazyLookup lkp = new LazyLookup(data, Lookup.EMPTY);
+
+        class LL implements LookupListener {
+            int cnt;
+            
+            @Override
+            public void resultChanged(LookupEvent ev) {
+                cnt++;
+                assertFalse("Internal lock isn't held", lkp.isInitializing());
+            }
+        }
+        LL listener = new LL();
+
+        Lookup.Result<String> res = lkp.lookupResult(String.class);
+        res.addLookupListener(listener);
+        
+        String value = lkp.lookup(String.class);
+        assertEquals("Hello", value);
+        
+        assertEquals("One change", 1, listener.cnt);
+    }
+    
+    public static String testFactory() {
+        return "Hello";
+    }
+}

ide/projectapi/test/unit/src/org/netbeans/spi/project/support/DelegatingLookupDeadlock5914Test.java

+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.netbeans.spi.project.support;
+
+import javax.swing.JButton;
+import javax.swing.JCheckBox;
+import javax.swing.JComboBox;
+import javax.swing.JRadioButton;
+import javax.swing.JTextArea;
+import javax.swing.JTextField;
+import org.netbeans.junit.NbTestCase;
+import org.netbeans.spi.project.LookupMerger;
+import org.netbeans.spi.project.LookupProvider;
+import org.openide.util.Lookup;
+import org.openide.util.LookupEvent;
+import org.openide.util.LookupListener;
+import org.openide.util.lookup.AbstractLookup;
+import org.openide.util.lookup.InstanceContent;
+import org.openide.util.lookup.Lookups;
+
+public class DelegatingLookupDeadlock5914Test extends NbTestCase {
+
+    public DelegatingLookupDeadlock5914Test(String name) {
+        super(name);
+    }
+
+    public void testDontHoldLockWhenNotifyingChanges() {
+        LookupMergerImpl merger = new LookupMergerImpl();
+        Lookup base = Lookups.fixed(new JButton(), new JComboBox(), merger);
+        LookupProviderImpl pro1 = new LookupProviderImpl();
+        LookupProviderImpl pro2 = new LookupProviderImpl();
+        LookupProviderImpl pro3 = new LookupProviderImpl();
+
+        InstanceContent provInst = new InstanceContent();
+        Lookup providers = new AbstractLookup(provInst);
+        provInst.add(pro1);
+        provInst.add(pro2);
+
+        pro1.ic.add(new JTextField());
+        pro2.ic.add(new JTextArea());
+
+        DelegatingLookupImpl del = new DelegatingLookupImpl(base, providers, "<irrelevant>");
+        class LL implements LookupListener {
+            int cnt;
+
+            @Override
+            public void resultChanged(LookupEvent ev) {
+                assertFalse("Cannot hold lock when notifying changes!", del.holdsLock());
+                cnt++;
+            }
+        }
+        LL jbuttonListener = new LL();
+        Lookup.Result<JButton> jbuttonResult = del.lookupResult(JButton.class);
+        jbuttonResult.addLookupListener(jbuttonListener);
+        assertEquals("One button", 1, jbuttonResult.allInstances().size());
+
+        Lookup.Result<JRadioButton> jradioButtonResult = del.lookupResult(JRadioButton.class);
+        LL jradioButtonListener = new LL();
+        jradioButtonResult.addLookupListener(jradioButtonListener);
+        assertEquals("No radio button", 0, jradioButtonResult.allInstances().size());
+
+        assertNotNull(del.lookup(JTextArea.class));
+        assertNotNull(del.lookup(JComboBox.class));
+
+        // test merger..