Partial fix for #172865: Don't hold global lock while seeking for getResources(...). That is expensive operation and can take long time. No need to block other threads.
authorJaroslav Tulach <jtulach@netbeans.org>
Thu, 24 Sep 2009 15:20:20 +0200
changeset 8265506760bcfbf
parent 825 7541c0f74ce0
child 827 9ef3d6e0c9e4
child 941 b50172b8c52f
Partial fix for #172865: Don't hold global lock while seeking for getResources(...). That is expensive operation and can take long time. No need to block other threads.
openide.util/src/org/openide/util/lookup/MetaInfServicesLookup.java
openide.util/test/unit/src/org/openide/util/lookup/MetaInfServicesLookupTest.java
     1.1 --- a/openide.util/src/org/openide/util/lookup/MetaInfServicesLookup.java	Wed Sep 23 13:31:43 2009 +0200
     1.2 +++ b/openide.util/src/org/openide/util/lookup/MetaInfServicesLookup.java	Thu Sep 24 15:20:20 2009 +0200
     1.3 @@ -113,15 +113,24 @@
     1.4      protected final void beforeLookup(Lookup.Template t) {
     1.5          Class c = t.getType();
     1.6  
     1.7 +        Collection<AbstractLookup.Pair<?>> toAdd = null;
     1.8 +        synchronized (this) {
     1.9 +            if (classes.get(c) == null) { // NOI18N
    1.10 +                toAdd = new ArrayList<Pair<?>>();
    1.11 +            } else {
    1.12 +                // ok, nothing needs to be done
    1.13 +                return;
    1.14 +            }
    1.15 +        }
    1.16 +        if (toAdd != null) {
    1.17 +            search(c, toAdd);
    1.18 +        }
    1.19          synchronized (this) {
    1.20              if (classes.put(c, "") == null) { // NOI18N
    1.21                  // Added new class, search for it.
    1.22                  LinkedHashSet<AbstractLookup.Pair<?>> arr = getPairsAsLHS();
    1.23 -                search(c, arr);
    1.24 +                arr.addAll(toAdd);
    1.25                  setPairs(arr, RP);
    1.26 -            } else {
    1.27 -                // ok, nothing needs to be done
    1.28 -                return;
    1.29              }
    1.30          }
    1.31      }
     2.1 --- a/openide.util/test/unit/src/org/openide/util/lookup/MetaInfServicesLookupTest.java	Wed Sep 23 13:31:43 2009 +0200
     2.2 +++ b/openide.util/test/unit/src/org/openide/util/lookup/MetaInfServicesLookupTest.java	Thu Sep 24 15:20:20 2009 +0200
     2.3 @@ -72,6 +72,7 @@
     2.4  import org.bar.Comparator2;
     2.5  import org.netbeans.junit.MockServices;
     2.6  import org.netbeans.junit.NbTestCase;
     2.7 +import org.openide.util.Exceptions;
     2.8  import org.openide.util.Lookup;
     2.9  import org.openide.util.LookupEvent;
    2.10  import org.openide.util.LookupListener;
    2.11 @@ -439,4 +440,56 @@
    2.12              fail("Collections are different:\nFirst: " + col1 + "\nLast:  " + col3);
    2.13          }
    2.14      }
    2.15 +
    2.16 +    public void testContentionWhenLoadingMetainfServices() throws Exception {
    2.17 +        class My extends ClassLoader implements Runnable {
    2.18 +            Lookup query;
    2.19 +            Integer value;
    2.20 +
    2.21 +            public void run() {
    2.22 +                value = query.lookup(Integer.class);
    2.23 +            }
    2.24 +
    2.25 +
    2.26 +            @Override
    2.27 +            protected URL findResource(String name) {
    2.28 +                waitForTask(name);
    2.29 +                return super.findResource(name);
    2.30 +            }
    2.31 +
    2.32 +            @Override
    2.33 +            protected Enumeration<URL> findResources(String name) throws IOException {
    2.34 +                waitForTask(name);
    2.35 +                return super.findResources(name);
    2.36 +            }
    2.37 +
    2.38 +            private synchronized void waitForTask(String name) {
    2.39 +                if (name.startsWith(prefix()) && Thread.currentThread().getName().contains("block")) {
    2.40 +                    try {
    2.41 +                        wait();
    2.42 +                    } catch (InterruptedException ex) {
    2.43 +                        Exceptions.printStackTrace(ex);
    2.44 +                    }
    2.45 +                }
    2.46 +            }
    2.47 +        }
    2.48 +
    2.49 +        My loader = new My();
    2.50 +        loader.query = createLookup(loader);
    2.51 +        Thread t = new Thread(loader, "block when querying");
    2.52 +        t.start();
    2.53 +        t.join(1000);
    2.54 +
    2.55 +        // this blocks waiting for the waitForTask to finish
    2.56 +        // right now
    2.57 +        Float f = loader.query.lookup(Float.class);
    2.58 +        assertNull("Nothing found", f);
    2.59 +
    2.60 +        synchronized (loader) {
    2.61 +            loader.notifyAll();
    2.62 +        }
    2.63 +        t.join();
    2.64 +
    2.65 +        assertNull("Nothing found", loader.value);
    2.66 +    }
    2.67  }