#119292: Making setLookups re-entrant from add/remove LookupListener post_nb6_dbcaching_root
authorjtulach@netbeans.org
Tue, 30 Oct 2007 17:51:00 +0000
changeset 307b799d75050fe
parent 306 66f0c21d2b83
child 308 0bbf3081605b
#119292: Making setLookups re-entrant from add/remove LookupListener
openide.util/src/org/openide/util/lookup/ProxyLookup.java
openide.util/test/unit/src/org/openide/util/lookup/ProxyLookupTest.java
     1.1 --- a/openide.util/src/org/openide/util/lookup/ProxyLookup.java	Tue Oct 30 17:33:24 2007 +0000
     1.2 +++ b/openide.util/src/org/openide/util/lookup/ProxyLookup.java	Tue Oct 30 17:51:00 2007 +0000
     1.3 @@ -155,6 +155,9 @@
     1.4          Set<Lookup> current;
     1.5          Lookup[] old;
     1.6          
     1.7 +        Map<Result,LookupListener> toRemove = new IdentityHashMap<Lookup.Result, LookupListener>();
     1.8 +        Map<Result,LookupListener> toAdd = new IdentityHashMap<Lookup.Result, LookupListener>();
     1.9 +        
    1.10          synchronized (this) {
    1.11              old = getLookups(false);
    1.12              current = identityHashSet(Arrays.asList(old));
    1.13 @@ -181,10 +184,19 @@
    1.14              for (Reference<R> ref : arr) {
    1.15                  R r = ref.get();
    1.16                  if (r != null) {
    1.17 -                    r.lookupChange(newL, removed, old, lookups);
    1.18 +                    r.lookupChange(newL, removed, old, lookups, toAdd, toRemove);
    1.19                  }
    1.20              }
    1.21          }
    1.22 +        
    1.23 +        // better to do this later than in synchronized block
    1.24 +        for (Map.Entry<Result, LookupListener> e : toRemove.entrySet()) {
    1.25 +            e.getKey().removeLookupListener(e.getValue());
    1.26 +        }
    1.27 +        for (Map.Entry<Result, LookupListener> e : toAdd.entrySet()) {
    1.28 +            e.getKey().addLookupListener(e.getValue());
    1.29 +        }
    1.30 +
    1.31  
    1.32          // this cannot be done from the synchronized block
    1.33          ArrayList<Object> evAndListeners = new ArrayList<Object>();
    1.34 @@ -362,7 +374,10 @@
    1.35           * @param remove set of removed lookups
    1.36           * @param current array of current lookups
    1.37           */
    1.38 -        protected void lookupChange(Set added, Set removed, Lookup[] old, Lookup[] current) {
    1.39 +        protected void lookupChange(
    1.40 +            Set added, Set removed, Lookup[] old, Lookup[] current, 
    1.41 +            Map<Result,LookupListener> toAdd, Map<Result,LookupListener> toRemove
    1.42 +        ) {
    1.43              synchronized (this) {
    1.44                  if (weakL.results == null) {
    1.45                      // not computed yet, do not need to do anything
    1.46 @@ -370,12 +385,12 @@
    1.47                  }
    1.48  
    1.49                  // map (Lookup, Lookup.Result)
    1.50 -                HashMap<Lookup,Result<T>> map = new HashMap<Lookup,Result<T>>(old.length * 2);
    1.51 +                Map<Lookup,Result<T>> map = new IdentityHashMap<Lookup,Result<T>>(old.length * 2);
    1.52  
    1.53                  for (int i = 0; i < old.length; i++) {
    1.54                      if (removed.contains(old[i])) {
    1.55                          // removed lookup
    1.56 -                        weakL.results[i].removeLookupListener(weakL);
    1.57 +                        toRemove.put(weakL.results[i], weakL);
    1.58                      } else {
    1.59                          // remember the association
    1.60                          map.put(old[i], weakL.results[i]);
    1.61 @@ -388,7 +403,7 @@
    1.62                      if (added.contains(current[i])) {
    1.63                          // new lookup
    1.64                          arr[i] = current[i].lookup(template);
    1.65 -                        arr[i].addLookupListener(weakL);
    1.66 +                        toAdd.put(arr[i], weakL);
    1.67                      } else {
    1.68                          // old lookup
    1.69                          arr[i] = map.get(current[i]);
     2.1 --- a/openide.util/test/unit/src/org/openide/util/lookup/ProxyLookupTest.java	Tue Oct 30 17:33:24 2007 +0000
     2.2 +++ b/openide.util/test/unit/src/org/openide/util/lookup/ProxyLookupTest.java	Tue Oct 30 17:51:00 2007 +0000
     2.3 @@ -62,7 +62,7 @@
     2.4      public static Test suite() {
     2.5          return new NbTestSuite (ProxyLookupTest.class);
     2.6          
     2.7 -//        return new ProxyLookupTest("testArrayIndexAsInIssue119292");
     2.8 +        //return new ProxyLookupTest("testArrayIndexWithAddRemoveListenerAsInIssue119292");
     2.9      }
    2.10      
    2.11      /** Creates an lookup for given lookup. This class just returns 
    2.12 @@ -414,4 +414,78 @@
    2.13          
    2.14          assertEquals("Still assigned to C", Collections.singletonList("C"), res.allInstances());
    2.15      }
    2.16 +    
    2.17 +    public void testArrayIndexWithAddRemoveListenerAsInIssue119292() throws Exception {
    2.18 +        final ProxyLookup pl = new ProxyLookup();
    2.19 +        final int[] cnt = { 0 };
    2.20 +        
    2.21 +        class L extends Lookup {
    2.22 +            L[] set;
    2.23 +            Lookup l;
    2.24 +            
    2.25 +            public L(String s) {
    2.26 +                l = Lookups.singleton(s);
    2.27 +            }
    2.28 +            
    2.29 +            @Override
    2.30 +            public <T> T lookup(Class<T> clazz) {
    2.31 +                return l.lookup(clazz);
    2.32 +            }
    2.33 +
    2.34 +            @Override
    2.35 +            public <T> Result<T> lookup(Template<T> template) {
    2.36 +                Result<T> r = l.lookup(template);
    2.37 +                return new R<T>(r);
    2.38 +            }
    2.39 +
    2.40 +            final class R<T> extends Result<T> {
    2.41 +                private Result<T> delegate;
    2.42 +
    2.43 +                public R(Result<T> delegate) {
    2.44 +                    this.delegate = delegate;
    2.45 +                }
    2.46 +                
    2.47 +                @Override
    2.48 +                public void addLookupListener(LookupListener l) {
    2.49 +                    cnt[0]++;
    2.50 +                    if (set != null) {
    2.51 +                        pl.setLookups(set);
    2.52 +                    }
    2.53 +                    delegate.addLookupListener(l);
    2.54 +                }
    2.55 +
    2.56 +                @Override
    2.57 +                public void removeLookupListener(LookupListener l) {
    2.58 +                    cnt[0]++;
    2.59 +                    if (set != null) {
    2.60 +                        pl.setLookups(set);
    2.61 +                    }
    2.62 +                    delegate.removeLookupListener(l);
    2.63 +                }
    2.64 +
    2.65 +                @Override
    2.66 +                public Collection<? extends T> allInstances() {
    2.67 +                    return delegate.allInstances();
    2.68 +                }
    2.69 +            }
    2.70 +        }
    2.71 +
    2.72 +        Result<String> res = pl.lookupResult(String.class);
    2.73 +        assertEquals(Collections.EMPTY_LIST, res.allItems());
    2.74 +        
    2.75 +        L[] old = { new L("A"), new L("B") };
    2.76 +        L[] now = { new L("C") };
    2.77 +        
    2.78 +        pl.setLookups(old);
    2.79 +        cnt[0] = 0;
    2.80 +        
    2.81 +        old[0].set = new L[0];
    2.82 +        pl.setLookups(now);
    2.83 +
    2.84 +        if (cnt[0] == 0) {
    2.85 +            fail("There should be calls to listeners");
    2.86 +        }
    2.87 +        
    2.88 +        assertEquals("C is overriden from removeLookupListener", Collections.emptyList(), res.allInstances());
    2.89 +    }
    2.90  }