FELIX-5034 : Reduce and correct locking related to the hook registry

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1703117 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
index 35cf90d..2f2aa5f 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleImpl.java
@@ -233,6 +233,7 @@
         m_activator = activator;
     }
 
+    @Override
     public BundleContext getBundleContext()
     {
         Object sm = System.getSecurityManager();
@@ -251,6 +252,7 @@
         m_context = context;
     }
 
+    @Override
     public long getBundleId()
     {
         try
@@ -268,6 +270,7 @@
         }
     }
 
+    @Override
     public URL getEntry(String name)
     {
         Object sm = System.getSecurityManager();
@@ -288,6 +291,7 @@
         return getFramework().getBundleEntry(this, name);
     }
 
+    @Override
     public Enumeration getEntryPaths(String path)
     {
         Object sm = System.getSecurityManager();
@@ -308,6 +312,7 @@
         return getFramework().getBundleEntryPaths(this, path);
     }
 
+    @Override
     public Enumeration findEntries(String path, String filePattern, boolean recurse)
     {
         Object sm = System.getSecurityManager();
@@ -329,11 +334,13 @@
                 this, path, filePattern, recurse);
     }
 
+    @Override
     public Dictionary getHeaders()
     {
         return getHeaders(Locale.getDefault().toString());
     }
 
+    @Override
     public Dictionary getHeaders(String locale)
     {
         Object sm = System.getSecurityManager();
@@ -569,6 +576,7 @@
         return result;
     }
 
+    @Override
     public long getLastModified()
     {
         try
@@ -602,6 +610,7 @@
         }
     }
 
+    @Override
     public String getLocation()
     {
         Object sm = System.getSecurityManager();
@@ -636,6 +645,7 @@
      *
      * @return a URL to named resource, or null if not found.
     **/
+    @Override
     public URL getResource(String name)
     {
         Object sm = System.getSecurityManager();
@@ -656,6 +666,7 @@
         return getFramework().getBundleResource(this, name);
     }
 
+    @Override
     public Enumeration getResources(String name) throws IOException
     {
         Object sm = System.getSecurityManager();
@@ -685,6 +696,7 @@
      *
      * @return an array of service references or null.
     **/
+    @Override
     public ServiceReference[] getRegisteredServices()
     {
         Object sm = System.getSecurityManager();
@@ -728,6 +740,7 @@
         }
     }
 
+    @Override
     public ServiceReference[] getServicesInUse()
     {
         Object sm = System.getSecurityManager();
@@ -769,6 +782,7 @@
         return getFramework().getBundleServicesInUse(this);
     }
 
+    @Override
     public int getState()
     {
         return m_state;
@@ -913,27 +927,32 @@
         return false;
     }
 
+    @Override
     public String getSymbolicName()
     {
         return adapt(BundleRevisionImpl.class).getSymbolicName();
     }
 
+    @Override
     public Version getVersion()
     {
         return adapt(BundleRevisionImpl.class).getVersion();
     }
 
+    @Override
     public boolean hasPermission(Object obj)
     {
         return getFramework().bundleHasPermission(this, obj);
     }
 
+    @Override
     public Map getSignerCertificates(int signersType)
     {
         // TODO: SECURITY - This needs to be adapted to our security mechanisms.
         return (Map) getFramework().getSignerMatcher(this, signersType);
     }
 
+    @Override
     public Class loadClass(String name) throws ClassNotFoundException
     {
         if (isExtension())
@@ -959,11 +978,13 @@
         return getFramework().loadBundleClass(this, name);
     }
 
+    @Override
     public void start() throws BundleException
     {
         start(0);
     }
 
+    @Override
     public void start(int options) throws BundleException
     {
         Object sm = System.getSecurityManager();
@@ -977,11 +998,13 @@
         getFramework().startBundle(this, options);
     }
 
+    @Override
     public void update() throws BundleException
     {
         update(null);
     }
 
+    @Override
     public void update(InputStream is) throws BundleException
     {
         Object sm = System.getSecurityManager();
@@ -995,11 +1018,13 @@
         getFramework().updateBundle(this, is);
     }
 
+    @Override
     public void stop() throws BundleException
     {
         stop(0);
     }
 
+    @Override
     public void stop(int options) throws BundleException
     {
         Object sm = System.getSecurityManager();
@@ -1013,6 +1038,7 @@
         getFramework().stopBundle(this, ((options & Bundle.STOP_TRANSIENT) == 0));
     }
 
+    @Override
     public void uninstall() throws BundleException
     {
         Object sm = System.getSecurityManager();
@@ -1064,6 +1090,7 @@
         }
     }
 
+    @Override
     public synchronized <A> A adapt(Class<A> type)
     {
         checkAdapt(type);
@@ -1125,11 +1152,13 @@
         return null;
     }
 
+    @Override
     public File getDataFile(String filename)
     {
         return getFramework().getDataFile(this, filename);
     }
 
+    @Override
     public int compareTo(Bundle t)
     {
         long thisBundleId = this.getBundleId();
@@ -1157,11 +1186,13 @@
     // Revision management.
     //
 
+    @Override
     public Bundle getBundle()
     {
         return this;
     }
 
+    @Override
     public synchronized List<BundleRevision> getRevisions()
     {
         return new ArrayList<BundleRevision>(m_revisions);
@@ -1280,7 +1311,7 @@
             }
             if (!collisionCanditates.isEmpty() && allowMultiple.equals(Constants.FRAMEWORK_BSNVERSION_MANAGED))
             {
-                Set<ServiceReference<CollisionHook>> hooks = getFramework().getHooks(CollisionHook.class);
+                Set<ServiceReference<CollisionHook>> hooks = getFramework().getHookRegistry().getHooks(CollisionHook.class);
                 if (!hooks.isEmpty())
                 {
                     Collection<Bundle> shrinkableCollisionCandidates = new ShrinkableCollection<Bundle>(collisionCanditates);
diff --git a/framework/src/main/java/org/apache/felix/framework/BundleWiringImpl.java b/framework/src/main/java/org/apache/felix/framework/BundleWiringImpl.java
index 80858b3..d092455 100644
--- a/framework/src/main/java/org/apache/felix/framework/BundleWiringImpl.java
+++ b/framework/src/main/java/org/apache/felix/framework/BundleWiringImpl.java
@@ -513,22 +513,26 @@
         return m_fragmentContents;
     }
 
+    @Override
     public boolean isCurrent()
     {
         BundleRevision current = getBundle().adapt(BundleRevision.class);
         return (current != null) && (current.getWiring() == this);
     }
 
+    @Override
     public synchronized boolean isInUse()
     {
         return !m_isDisposed;
     }
 
+    @Override
     public List<Capability> getResourceCapabilities(String namespace)
     {
         return BundleRevisionImpl.asCapabilityList(getCapabilities(namespace));
     }
 
+    @Override
     public List<BundleCapability> getCapabilities(String namespace)
     {
         if (isInUse())
@@ -550,11 +554,13 @@
         return null;
     }
 
+    @Override
     public List<Requirement> getResourceRequirements(String namespace)
     {
         return BundleRevisionImpl.asRequirementList(getRequirements(namespace));
     }
 
+    @Override
     public List<BundleRequirement> getRequirements(String namespace)
     {
         if (isInUse())
@@ -596,11 +602,13 @@
         return wires;
     }
 
+    @Override
     public List<Wire> getProvidedResourceWires(String namespace)
     {
         return asWireList(getProvidedWires(namespace));
     }
 
+    @Override
     public List<BundleWire> getProvidedWires(String namespace)
     {
         if (isInUse())
@@ -611,11 +619,13 @@
         return null;
     }
 
+    @Override
     public List<Wire> getRequiredResourceWires(String namespace)
     {
         return asWireList(getRequiredWires(namespace));
     }
 
+    @Override
     public List<BundleWire> getRequiredWires(String namespace)
     {
         if (isInUse())
@@ -656,16 +666,19 @@
         m_importedPkgs = importedPkgs;
     }
 
+    @Override
     public BundleRevision getResource()
     {
         return m_revision;
     }
 
+    @Override
     public BundleRevision getRevision()
     {
         return m_revision;
     }
 
+    @Override
     public ClassLoader getClassLoader()
     {
         if (m_isDisposed)
@@ -723,6 +736,7 @@
         return m_classLoader;
     }
 
+    @Override
     public List<URL> findEntries(String path, String filePattern, int options)
     {
         if (isInUse())
@@ -749,6 +763,7 @@
     private final ThreadLocal m_listResourcesCycleCheck = new ThreadLocal();
 
 // TODO: OSGi R4.3 - Should this be synchronized or should we take a snapshot?
+    @Override
     public synchronized Collection<String> listResources(
         String path, String filePattern, int options)
     {
@@ -1056,6 +1071,7 @@
         return SimpleFilter.compareSubstring(pattern, resource);
     }
 
+    @Override
     public Bundle getBundle()
     {
         return m_revision.getBundle();
@@ -1688,6 +1704,7 @@
                     return AccessController
                         .doPrivileged(new PrivilegedExceptionAction()
                         {
+                            @Override
                             public Object run() throws Exception
                             {
                                 return doImplicitBootDelegation(classes, name,
@@ -1851,11 +1868,13 @@
             m_enumeration = enumeration;
         }
 
+        @Override
         public boolean hasMoreElements()
         {
             return m_enumeration.hasMoreElements();
         }
 
+        @Override
         public Object nextElement()
         {
             return convertToLocalUrl((URL) m_enumeration.nextElement());
@@ -1973,6 +1992,7 @@
             return m_isActivationTriggered;
         }
 
+        @Override
         public Bundle getBundle()
         {
             return m_wiring.getBundle();
@@ -2077,10 +2097,10 @@
                     Felix felix = ((BundleImpl) m_wiring.m_revision.getBundle()).getFramework();
 
                     Set<ServiceReference<WeavingHook>> hooks =
-                        felix.getHooks(WeavingHook.class);
+                        felix.getHookRegistry().getHooks(WeavingHook.class);
 
                     Set<ServiceReference<WovenClassListener>> wovenClassListeners =
-                        felix.getHooks(WovenClassListener.class);
+                        felix.getHookRegistry().getHooks(WovenClassListener.class);
 
                     WovenClassImpl wci = null;
                     if (!hooks.isEmpty())
@@ -2390,7 +2410,7 @@
             for (ServiceReference<WeavingHook> sr : hooks)
             {
                 // Only use the hook if it is not black listed.
-                if (!felix.isHookBlackListed(sr))
+                if (!felix.getHookRegistry().isHookBlackListed(sr))
                 {
                     // Get the hook service object.
                     // Note that we don't use the bundle context
@@ -2408,7 +2428,7 @@
                         {
                             if (!(th instanceof WeavingException))
                             {
-                                felix.blackListHook(sr);
+                                felix.getHookRegistry().blackListHook(sr);
                             }
                             felix.fireFrameworkEvent(
                                     FrameworkEvent.ERROR,
@@ -2666,6 +2686,7 @@
             return m_resource.hashCode();
         }
 
+        @Override
         public int compareTo(ResourceSource t)
         {
             return m_resource.compareTo(t.m_resource);
diff --git a/framework/src/main/java/org/apache/felix/framework/Felix.java b/framework/src/main/java/org/apache/felix/framework/Felix.java
index b8fc55e..c675ff7 100644
--- a/framework/src/main/java/org/apache/felix/framework/Felix.java
+++ b/framework/src/main/java/org/apache/felix/framework/Felix.java
@@ -118,9 +118,9 @@
     // Logging related member variables.
     private final Logger m_logger;
     // Immutable config properties.
-    private final Map m_configMap;
+    private final Map<String, Object> m_configMap;
     // Mutable configuration properties passed into constructor.
-    private final Map m_configMutableMap;
+    private final Map<String, Object> m_configMutableMap;
 
     // Resolver and resolver state.
     private final StatefulResolver m_resolver;
@@ -129,7 +129,7 @@
     // lock or the global lock can be acquired.
     private final Object[] m_bundleLock = new Object[0];
     // Keeps track of threads wanting to acquire the global lock.
-    private final List m_globalLockWaitersList = new ArrayList();
+    private final List<Thread> m_globalLockWaitersList = new ArrayList<Thread>();
     // The thread currently holding the global lock.
     private Thread m_globalLockThread = null;
     // How many times the global lock was acquired by the thread holding
@@ -139,7 +139,7 @@
 
     // Maps a bundle location to a bundle location;
     // used to reserve a location when installing a bundle.
-    private final Map m_installRequestMap = new HashMap();
+    private final Map<String, String> m_installRequestMap = new HashMap<String, String>();
     // This lock must be acquired to modify m_installRequestMap;
     // to help avoid deadlock this lock as priority 1 and should
     // be acquired before locks with lower priority.
@@ -164,7 +164,7 @@
     private volatile int m_activeStartLevel = FelixConstants.FRAMEWORK_INACTIVE_STARTLEVEL;
     // Framework's target start level.
     // Normally the target start will equal the active start level, except
-    // whem the start level is changing, in which case the target start level
+    // when the start level is changing, in which case the target start level
     // will report the new start level while the active start level will report
     // the old start level. Once the start level change is complete, the two
     // will become equal again.
@@ -414,6 +414,7 @@
 
         // Create service registry.
         m_registry = new ServiceRegistry(m_logger, new ServiceRegistryCallbacks() {
+            @Override
             public void serviceChanged(ServiceEvent event, Dictionary oldProps)
             {
                 fireServiceEvent(event, oldProps);
@@ -619,6 +620,7 @@
     }
 
 
+    @Override
     public void init() throws BundleException
     {
         init((FrameworkListener[]) null);
@@ -626,6 +628,7 @@
     /**
      * @see org.osgi.framework.launch.Framework#init(org.osgi.framework.FrameworkListener[])
      */
+    @Override
     public void init(final FrameworkListener... listeners) throws BundleException
     {
         // The system bundle can only be initialized if it currently isn't started.
@@ -1021,6 +1024,7 @@
             // Spec says stop() on SystemBundle should return immediately and
             // shutdown framework on another thread.
             new Thread(new Runnable() {
+                @Override
                 public void run()
                 {
                     try
@@ -1051,6 +1055,7 @@
      * @param timeout A timeout value.
      * @throws java.lang.InterruptedException If the thread was interrupted.
     **/
+    @Override
     public FrameworkEvent waitForStop(long timeout) throws InterruptedException
     {
         // Throw exception if timeout is negative.
@@ -1119,6 +1124,7 @@
 
         // Then to stop and restart the framework on a separate thread.
         new Thread(new Runnable() {
+            @Override
             public void run()
             {
                 try
@@ -3106,7 +3112,7 @@
         if (existing != null)
         {
             Set<ServiceReference<org.osgi.framework.hooks.bundle.FindHook>> hooks =
-                getHooks(org.osgi.framework.hooks.bundle.FindHook.class);
+                    getHookRegistry().getBundleFindHooks();
             if (!hooks.isEmpty())
             {
                 Collection<Bundle> bundles = new ArrayList<Bundle>(1);
@@ -3200,7 +3206,7 @@
         }
 
         Set<ServiceReference<org.osgi.framework.hooks.bundle.FindHook>> hooks =
-            getHooks(org.osgi.framework.hooks.bundle.FindHook.class);
+                getHookRegistry().getBundleFindHooks();
         if (!hooks.isEmpty() && (bundle != null))
         {
             Collection<Bundle> bundles = new ArrayList<Bundle>(1);
@@ -3268,44 +3274,42 @@
      * Implementation for BundleContext.getBundles(). Retrieves
      * all installed bundles.
      *
-     * @return An array containing all installed bundles or null if
-     *         there are no installed bundles.
-    **/
+     * @return An array containing all installed bundles or an empty
+     *        array if there are no installed bundles.
+     **/
     Bundle[] getBundles(BundleContext bc)
     {
         Collection<Bundle> bundles = m_installedBundles[IDENTIFIER_MAP_IDX].values();
-        Set<ServiceReference<org.osgi.framework.hooks.bundle.FindHook>> hooks =
-            getHooks(org.osgi.framework.hooks.bundle.FindHook.class);
-        if (!hooks.isEmpty())
+        // If the requesting bundle is something other than the system bundle, return the shrunk
+        // collection of bundles. If it *is* the system bundle, it should receive the unfiltered bundles.
+        if ( !bundles.isEmpty() && bc.getBundle() != this )
         {
-            Collection<Bundle> shrunkBundles = new ShrinkableCollection<Bundle>(new ArrayList(bundles));
-            for (ServiceReference<org.osgi.framework.hooks.bundle.FindHook> hook : hooks)
+            Set<ServiceReference<org.osgi.framework.hooks.bundle.FindHook>> hooks =
+                    getHookRegistry().getBundleFindHooks();
+            if (!hooks.isEmpty())
             {
-                org.osgi.framework.hooks.bundle.FindHook fh = getService(this, hook, false);
-                if (fh != null)
+                Collection<Bundle> shrunkBundles = new ShrinkableCollection<Bundle>(new ArrayList<Bundle>(bundles));
+                for (ServiceReference<org.osgi.framework.hooks.bundle.FindHook> hook : hooks)
                 {
-                    try
+                    org.osgi.framework.hooks.bundle.FindHook fh = getService(this, hook, false);
+                    if (fh != null)
                     {
-                        m_secureAction.invokeBundleFindHook(fh, bc, shrunkBundles);
-                    }
-                    catch (Throwable th)
-                    {
-                        m_logger.doLog(
-                            hook.getBundle(),
-                            hook,
-                            Logger.LOG_WARNING,
-                            "Problem invoking bundle hook.",
-                            th);
+                        try
+                        {
+                            m_secureAction.invokeBundleFindHook(fh, bc, shrunkBundles);
+                        }
+                        catch (Throwable th)
+                        {
+                            m_logger.doLog(
+                                hook.getBundle(),
+                                hook,
+                                Logger.LOG_WARNING,
+                                "Problem invoking bundle hook.",
+                                th);
+                        }
                     }
                 }
             }
-
-            if (bc.getBundle() != this)
-            {
-                // If the requesting bundle is something other than the system bundle, return the shrunk
-                // collection of bundles. If it *is* the system bundle, it should receive the unfiltered bundles.
-                bundles = shrunkBundles;
-            }
         }
 
         return bundles.toArray(new Bundle[bundles.size()]);
@@ -3355,7 +3359,7 @@
 
         // Invoke ListenerHook.removed() if filter updated.
         Set<ServiceReference<org.osgi.framework.hooks.service.ListenerHook>> listenerHooks =
-            m_registry.getHookRegistry().getHooks(org.osgi.framework.hooks.service.ListenerHook.class);
+                getHookRegistry().getServiceListenerHooks();
         if (oldFilter != null)
         {
             final Collection removed = Collections.singleton(
@@ -3426,7 +3430,7 @@
         {
             // Invoke the ListenerHook.removed() on all hooks.
             Set<ServiceReference<org.osgi.framework.hooks.service.ListenerHook>> listenerHooks =
-                m_registry.getHookRegistry().getHooks(org.osgi.framework.hooks.service.ListenerHook.class);
+                    getHookRegistry().getServiceListenerHooks();
             Collection removed = Collections.singleton(listener);
             for (ServiceReference<org.osgi.framework.hooks.service.ListenerHook> sr : listenerHooks)
             {
@@ -3597,7 +3601,7 @@
 
         // activate findhooks
         Set<ServiceReference<org.osgi.framework.hooks.service.FindHook>> findHooks =
-            m_registry.getHookRegistry().getHooks(org.osgi.framework.hooks.service.FindHook.class);
+                getHookRegistry().getServiceFindHooks();
         for (ServiceReference<org.osgi.framework.hooks.service.FindHook> sr : findHooks)
         {
             org.osgi.framework.hooks.service.FindHook fh = getService(this, sr, false);
@@ -3734,19 +3738,9 @@
     // Hook service management methods.
     //
 
-    boolean isHookBlackListed(final ServiceReference sr)
+    HookRegistry getHookRegistry()
     {
-        return m_registry.getHookRegistry().isHookBlackListed(sr);
-    }
-
-    void blackListHook(final ServiceReference sr)
-    {
-        m_registry.getHookRegistry().blackListHook(sr);
-    }
-
-    public <S> Set<ServiceReference<S>> getHooks(final Class<S> hookClass)
-    {
-        return m_registry.getHookRegistry().getHooks(hookClass);
+        return m_registry.getHookRegistry();
     }
 
     //
@@ -4828,6 +4822,7 @@
 
     class SystemBundleActivator implements BundleActivator
     {
+        @Override
         public void start(BundleContext context) throws Exception
         {
             // Add the bundle activator for the url handler service.
@@ -4854,6 +4849,7 @@
             }
         }
 
+        @Override
         public void stop(BundleContext context)
         {
             // The state of the framework should be STOPPING, so
@@ -5084,6 +5080,7 @@
             m_level = level;
         }
 
+        @Override
         public int compareTo(StartLevelTuple t)
         {
             int result = 1;
diff --git a/framework/src/main/java/org/apache/felix/framework/HookRegistry.java b/framework/src/main/java/org/apache/felix/framework/HookRegistry.java
index 0814948..3f06052 100644
--- a/framework/src/main/java/org/apache/felix/framework/HookRegistry.java
+++ b/framework/src/main/java/org/apache/felix/framework/HookRegistry.java
@@ -18,10 +18,8 @@
  */
 package org.apache.felix.framework;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.SortedSet;
@@ -60,8 +58,8 @@
         HOOK_CLASSES.put(c.getName(), c);
     }
 
-    private final Map<String, Set<ServiceReference<?>>> m_allHooks =
-        new HashMap<String, Set<ServiceReference<?>>>();
+    private final Map<String, SortedSet<ServiceReference<?>>> m_allHooks =
+        new HashMap<String, SortedSet<ServiceReference<?>>>();
 
     private final WeakHashMap<ServiceReference<?>, ServiceReference<?>> m_blackList =
             new WeakHashMap<ServiceReference<?>, ServiceReference<?>>();
@@ -109,6 +107,12 @@
         return false;
     }
 
+    /**
+     * Check and add the service to the set of hooks
+     * @param classNames The service names
+     * @param svcObj The service object
+     * @param ref The service reference
+     */
     public void addHooks(final String[] classNames, final Object svcObj, final ServiceReference<?> ref)
     {
         for(final String serviceName : classNames)
@@ -117,18 +121,26 @@
             {
                 synchronized (m_allHooks)
                 {
-                    Set<ServiceReference<?>> hooks = m_allHooks.get(serviceName);
+                    SortedSet<ServiceReference<?>> hooks = m_allHooks.get(serviceName);
                     if (hooks == null)
                     {
                         hooks = new TreeSet<ServiceReference<?>>(Collections.reverseOrder());
-                        m_allHooks.put(serviceName, hooks);
+                    }
+                    else
+                    {
+                        hooks = new TreeSet<ServiceReference<?>>(hooks);
                     }
                     hooks.add(ref);
+                    m_allHooks.put(serviceName, hooks);
                 }
             }
         }
     }
 
+    /**
+     * Update the service ranking for a hook
+     * @param ref The service reference
+     */
     public void updateHooks(final ServiceReference<?> ref)
     {
         // We maintain the hooks sorted, so if ranking has changed for example,
@@ -143,18 +155,21 @@
             {
                 synchronized (m_allHooks)
                 {
-                    final Set<ServiceReference<?>> hooks = m_allHooks.get(serviceName);
+                    SortedSet<ServiceReference<?>> hooks = m_allHooks.get(serviceName);
                     if (hooks != null)
                     {
-                        List<ServiceReference<?>> refs = new ArrayList<ServiceReference<?>>(hooks);
-                        hooks.clear();
-                        hooks.addAll(refs);
+                        hooks = new TreeSet<ServiceReference<?>>(hooks);
+                        m_allHooks.put(serviceName, hooks);
                     }
                 }
             }
         }
     }
 
+    /**
+     * Remove the service hooks
+     * @param ref The service reference
+     */
     public void removeHooks(final ServiceReference<?> ref)
     {
         final Object svcObj = ((ServiceRegistrationImpl.ServiceReferenceImpl) ref)
@@ -167,44 +182,72 @@
             {
                 synchronized (m_allHooks)
                 {
-                    final Set<ServiceReference<?>> hooks = m_allHooks.get(serviceName);
+                    SortedSet<ServiceReference<?>> hooks = m_allHooks.get(serviceName);
                     if (hooks != null)
                     {
+                        hooks = new TreeSet<ServiceReference<?>>(hooks);
                         hooks.remove(ref);
-                        if (hooks.isEmpty())
-                        {
-                            m_allHooks.remove(serviceName);
-                        }
+                        m_allHooks.put(serviceName, hooks);
                     }
                 }
             }
         }
-        m_blackList.remove(ref);
+        synchronized ( m_blackList )
+        {
+            m_blackList.remove(ref);
+        }
     }
 
+    /**
+     * Return the sorted set of hooks
+     * @param hookClass The hook class
+     * @return The sorted set - the set might be empty
+     */
+    @SuppressWarnings({ "unchecked", "rawtypes" })
     public <S> Set<ServiceReference<S>> getHooks(final Class<S> hookClass)
     {
-        synchronized (m_allHooks)
+        final Set<ServiceReference<?>> hooks = m_allHooks.get(hookClass.getName());
+        if (hooks != null)
         {
-            final Set<ServiceReference<?>> hooks = m_allHooks.get(hookClass.getName());
-            if (hooks != null)
-            {
-                SortedSet<ServiceReference<?>> sorted = new TreeSet<ServiceReference<?>>(Collections.reverseOrder());
-                sorted.addAll(hooks);
-                return (Set) sorted;
-            }
-            return Collections.emptySet();
+            return (Set)hooks;
         }
+        return Collections.emptySet();
+    }
+
+    public Set<ServiceReference<org.osgi.framework.hooks.bundle.FindHook>> getBundleFindHooks()
+    {
+        return getHooks(org.osgi.framework.hooks.bundle.FindHook.class);
+    }
+
+    public Set<ServiceReference<org.osgi.framework.hooks.service.FindHook>> getServiceFindHooks()
+    {
+        return getHooks(org.osgi.framework.hooks.service.FindHook.class);
+    }
+
+    public Set<ServiceReference<org.osgi.framework.hooks.bundle.EventHook>> getBundleEventHooks()
+    {
+        return getHooks(org.osgi.framework.hooks.bundle.EventHook.class);
+    }
+
+    public Set<ServiceReference<org.osgi.framework.hooks.service.ListenerHook>> getServiceListenerHooks()
+    {
+        return getHooks(org.osgi.framework.hooks.service.ListenerHook.class);
     }
 
     public boolean isHookBlackListed(final ServiceReference<?> sr)
     {
-        return m_blackList.containsKey(sr);
+        synchronized ( m_blackList )
+        {
+            return m_blackList.containsKey(sr);
+        }
     }
 
     public void blackListHook(final ServiceReference<?> sr)
     {
-        m_blackList.put(sr, sr);
+        synchronized ( m_blackList )
+        {
+            m_blackList.put(sr, sr);
+        }
     }
 
 }
diff --git a/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java b/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java
index de37097..35db9f7 100644
--- a/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java
+++ b/framework/src/main/java/org/apache/felix/framework/StatefulResolver.java
@@ -668,7 +668,7 @@
 
         // Get resolver hook factories.
         Set<ServiceReference<ResolverHookFactory>> hookRefs =
-            m_felix.getHooks(ResolverHookFactory.class);
+            m_felix.getHookRegistry().getHooks(ResolverHookFactory.class);
         Collection<BundleRevision> whitelist;
 
         if (!hookRefs.isEmpty())
@@ -1718,6 +1718,7 @@
         {
             return new Iterable<ResolverHook>()
             {
+                @Override
                 public Iterator<ResolverHook> iterator()
                 {
                     return new Iterator<ResolverHook>()
@@ -1726,6 +1727,7 @@
                             m_resolveHookMap.entrySet().iterator();
                         private Entry<ServiceReference<ResolverHookFactory>, ResolverHook> next = null;
 
+                        @Override
                         public boolean hasNext()
                         {
                             if (next == null)
@@ -1734,6 +1736,7 @@
                             return next != null;
                         }
 
+                        @Override
                         public ResolverHook next()
                         {
                             if (next == null)
@@ -1761,6 +1764,7 @@
                             }
                         }
 
+                        @Override
                         public void remove()
                         {
                             throw new UnsupportedOperationException();
diff --git a/framework/src/main/java/org/apache/felix/framework/URLHandlersActivator.java b/framework/src/main/java/org/apache/felix/framework/URLHandlersActivator.java
index d8dea4a..63101aa 100644
--- a/framework/src/main/java/org/apache/felix/framework/URLHandlersActivator.java
+++ b/framework/src/main/java/org/apache/felix/framework/URLHandlersActivator.java
@@ -52,6 +52,7 @@
     // Bundle activator methods.
     //
 
+    @Override
     public void start(BundleContext context)
     {
         // Only register the framework with the URL Handlers service
@@ -68,6 +69,7 @@
         URLHandlers.registerFrameworkInstance(m_framework, enable);
     }
 
+    @Override
     public void stop(BundleContext context)
     {
         URLHandlers.unregisterFrameworkInstance(m_framework);
@@ -77,14 +79,14 @@
     protected Object getStreamHandlerService(String protocol)
     {
         return get(
-            m_framework.getHooks(URLStreamHandlerService.class),
+            m_framework.getHookRegistry().getHooks(URLStreamHandlerService.class),
             "url.handler.protocol", protocol);
     }
 
     protected Object getContentHandlerService(String mimeType)
     {
         return get(
-            m_framework.getHooks(ContentHandler.class),
+            m_framework.getHookRegistry().getHooks(ContentHandler.class),
             "url.content.mimetype", mimeType);
     }
 
diff --git a/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java b/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
index 48208bf..c3d02bc 100644
--- a/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
+++ b/framework/src/main/java/org/apache/felix/framework/util/EventDispatcher.java
@@ -95,6 +95,7 @@
                 m_stopping = false;
 
                 m_thread = new Thread(new Runnable() {
+                    @Override
                     public void run()
                     {
                         try
@@ -552,7 +553,7 @@
     {
         Set<ServiceReference<org.osgi.framework.hooks.service.EventHook>> ehs =
             m_registry.getHookRegistry().getHooks(org.osgi.framework.hooks.service.EventHook.class);
-        if ((ehs != null) && !ehs.isEmpty())
+        if (!ehs.isEmpty())
         {
             // Create a whitelist of bundle context for bundle listeners,
             // if we have hooks.
@@ -575,7 +576,7 @@
 
         Set<ServiceReference<org.osgi.framework.hooks.service.EventListenerHook>> elhs =
             m_registry.getHookRegistry().getHooks(org.osgi.framework.hooks.service.EventListenerHook.class);
-        if ((elhs != null) && !elhs.isEmpty())
+        if (!elhs.isEmpty())
         {
             List<ListenerInfo> systemBundleListeners = null;
 
@@ -677,7 +678,7 @@
         // Create a whitelist of bundle context, if we have hooks.
         Set<BundleContext> whitelist = null;
         Set<ServiceReference<T>> hooks = m_registry.getHookRegistry().getHooks(hookClass);
-        if ((hooks != null) && !hooks.isEmpty())
+        if (!hooks.isEmpty())
         {
             boolean systemBundleListener = false;
             BundleContext systemBundleContext = felix.getBundleContext();
@@ -868,6 +869,7 @@
             if (System.getSecurityManager() != null)
             {
                 AccessController.doPrivileged(new PrivilegedAction() {
+                    @Override
                     public Object run()
                     {
                         ((FrameworkListener) l).frameworkEvent((FrameworkEvent) event);
@@ -901,6 +903,7 @@
             if (System.getSecurityManager() != null)
             {
                 AccessController.doPrivileged(new PrivilegedAction() {
+                    @Override
                     public Object run()
                     {
                         ((BundleListener) l).bundleChanged((BundleEvent) event);
@@ -975,6 +978,7 @@
                     {
                         AccessController.doPrivileged(new PrivilegedAction()
                         {
+                            @Override
                             public Object run()
                             {
                                 ((ServiceListener) l).serviceChanged((ServiceEvent) event);
@@ -1001,6 +1005,7 @@
                     {
                         AccessController.doPrivileged(new PrivilegedAction()
                         {
+                            @Override
                             public Object run()
                             {
                                 ((ServiceListener) l).serviceChanged(se);
diff --git a/framework/src/main/java/org/apache/felix/framework/util/ShrinkableCollection.java b/framework/src/main/java/org/apache/felix/framework/util/ShrinkableCollection.java
index 4d9bd96..6a6ad6b 100644
--- a/framework/src/main/java/org/apache/felix/framework/util/ShrinkableCollection.java
+++ b/framework/src/main/java/org/apache/felix/framework/util/ShrinkableCollection.java
@@ -33,26 +33,31 @@
         m_delegate = delegate;
     }
 
+    @Override
     public boolean add(T o)
     {
         throw new UnsupportedOperationException();
     }
 
+    @Override
     public boolean addAll(Collection<? extends T> c)
     {
         throw new UnsupportedOperationException();
     }
 
+    @Override
     public void clear()
     {
         m_delegate.clear();
     }
 
+    @Override
     public boolean contains(Object o)
     {
         return m_delegate.contains(o);
     }
 
+    @Override
     public boolean containsAll(Collection<?> c)
     {
         return m_delegate.containsAll(c);
@@ -70,41 +75,49 @@
         return m_delegate.hashCode();
     }
 
+    @Override
     public boolean isEmpty()
     {
         return m_delegate.isEmpty();
     }
 
-    public Iterator iterator()
+    @Override
+    public Iterator<T> iterator()
     {
         return m_delegate.iterator();
     }
 
+    @Override
     public boolean remove(Object o)
     {
         return m_delegate.remove(o);
     }
 
+    @Override
     public boolean removeAll(Collection<?> c)
     {
         return m_delegate.removeAll(c);
     }
 
+    @Override
     public boolean retainAll(Collection<?> c)
     {
         return m_delegate.retainAll(c);
     }
 
+    @Override
     public int size()
     {
         return m_delegate.size();
     }
 
+    @Override
     public Object[] toArray()
     {
         return m_delegate.toArray();
     }
 
+    @Override
     public <A> A[] toArray(A[] a)
     {
         return m_delegate.toArray(a);
diff --git a/framework/src/test/java/org/apache/felix/framework/BundleWiringImplTest.java b/framework/src/test/java/org/apache/felix/framework/BundleWiringImplTest.java
index 9f301aa..fc7728a 100644
--- a/framework/src/test/java/org/apache/felix/framework/BundleWiringImplTest.java
+++ b/framework/src/test/java/org/apache/felix/framework/BundleWiringImplTest.java
@@ -18,8 +18,12 @@
  */

 package org.apache.felix.framework;

 

-import static org.junit.Assert.*;

-import static org.mockito.Mockito.*;

+import static org.junit.Assert.assertEquals;

+import static org.junit.Assert.assertNotNull;

+import static org.junit.Assert.assertNull;

+import static org.junit.Assert.fail;

+import static org.mockito.Mockito.mock;

+import static org.mockito.Mockito.when;

 

 import java.io.ByteArrayOutputStream;

 import java.io.IOException;

@@ -36,6 +40,7 @@
 import org.apache.felix.framework.BundleWiringImpl.BundleClassLoaderJava5;

 import org.apache.felix.framework.cache.Content;

 import org.junit.Test;

+import org.mockito.Mockito;

 import org.objectweb.asm.ClassReader;

 import org.objectweb.asm.ClassWriter;

 import org.objectweb.asm.Opcodes;

@@ -126,6 +131,8 @@
     public void testFindClassExistant() throws Exception

     {

         Felix mockFramework = mock(Felix.class);

+        HookRegistry hReg = mock(HookRegistry.class);

+        Mockito.when(mockFramework.getHookRegistry()).thenReturn(hReg);

         Content mockContent = mock(Content.class);

         Class testClass = TestClass.class;

         String testClassName = testClass.getName();

@@ -191,13 +198,15 @@
         when(mockContent.getEntryAsBytes(testClassAsPath)).thenReturn(

                 testClassBytes);

 

-        when(mockFramework.getHooks(WeavingHook.class)).thenReturn(hooks);

+        HookRegistry hReg = mock(HookRegistry.class);

+        when(hReg.getHooks(WeavingHook.class)).thenReturn(hooks);

+        when(mockFramework.getHookRegistry()).thenReturn(hReg);

         when(

                 mockFramework.getService(mockFramework,

                         mockServiceReferenceWeavingHook, false)).thenReturn(

                 new GoodDummyWovenHook());

 

-        when(mockFramework.getHooks(WovenClassListener.class)).thenReturn(

+        when(hReg.getHooks(WovenClassListener.class)).thenReturn(

                 listeners);

         when(

                 mockFramework.getService(mockFramework,

@@ -261,13 +270,15 @@
         when(mockContent.getEntryAsBytes(testClassAsPath)).thenReturn(

                 testClassBytes);

 

-        when(mockFramework.getHooks(WeavingHook.class)).thenReturn(hooks);

+        HookRegistry hReg = mock(HookRegistry.class);

+        when(hReg.getHooks(WeavingHook.class)).thenReturn(hooks);

+        when(mockFramework.getHookRegistry()).thenReturn(hReg);

         when(

                 mockFramework.getService(mockFramework,

                         mockServiceReferenceWeavingHook, false)).thenReturn(

                 new BadDummyWovenHook());

 

-        when(mockFramework.getHooks(WovenClassListener.class)).thenReturn(

+        when(hReg.getHooks(WovenClassListener.class)).thenReturn(

                 listeners);

         when(

                 mockFramework.getService(mockFramework,

@@ -330,13 +341,15 @@
         when(mockContent.getEntryAsBytes(testClassAsPath)).thenReturn(

                 testClassBytes);

 

-        when(mockFramework.getHooks(WeavingHook.class)).thenReturn(hooks);

+        HookRegistry hReg = mock(HookRegistry.class);

+        when(hReg.getHooks(WeavingHook.class)).thenReturn(hooks);

+        when(mockFramework.getHookRegistry()).thenReturn(hReg);

         when(

                 mockFramework.getService(mockFramework,

                         mockServiceReferenceWeavingHook, false)).thenReturn(

                 new BadDefineWovenHook());

 

-        when(mockFramework.getHooks(WovenClassListener.class)).thenReturn(

+        when(hReg.getHooks(WovenClassListener.class)).thenReturn(

                 listeners);

         when(

                 mockFramework.getService(mockFramework,

@@ -353,7 +366,7 @@
             fail("Class should throw exception");

         } catch (Throwable e)

         {

-            

+

         }

         assertEquals("There should be 2 state changes fired by the weaving", 2,

                 dummyWovenClassListener.stateList.size());

@@ -408,6 +421,7 @@
     class GoodDummyWovenHook implements WeavingHook

     {

         // Adds the awesomePublicField to a class

+        @Override

         @SuppressWarnings("unchecked")

         public void weave(WovenClass wovenClass)

         {

@@ -426,6 +440,7 @@
     class BadDefineWovenHook implements WeavingHook

     {

         // Adds the awesomePublicField twice to the class. This is bad java.

+        @Override

         @SuppressWarnings("unchecked")

         public void weave(WovenClass wovenClass)

         {

@@ -446,6 +461,7 @@
     class BadDummyWovenHook implements WeavingHook

     {

         // Just Blow up

+        @Override

         public void weave(WovenClass wovenClass)

         {

             throw new WeavingException("Bad Weaver!");

@@ -456,6 +472,7 @@
     {

         public List<Integer> stateList = new ArrayList<Integer>();

 

+        @Override

         public void modified(WovenClass wovenClass)

         {

             stateList.add(wovenClass.getState());

diff --git a/framework/src/test/java/org/apache/felix/framework/CollisionHookTest.java b/framework/src/test/java/org/apache/felix/framework/CollisionHookTest.java
index 63496a5..9cc6823 100644
--- a/framework/src/test/java/org/apache/felix/framework/CollisionHookTest.java
+++ b/framework/src/test/java/org/apache/felix/framework/CollisionHookTest.java
@@ -18,13 +18,14 @@
  */
 package org.apache.felix.framework;
 
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.when;
+
 import java.util.Collection;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 
-import junit.framework.TestCase;
-
 import org.apache.felix.framework.cache.BundleArchive;
 import org.apache.felix.framework.cache.BundleArchiveRevision;
 import org.mockito.Mockito;
@@ -35,6 +36,8 @@
 import org.osgi.framework.Version;
 import org.osgi.framework.hooks.bundle.CollisionHook;
 
+import junit.framework.TestCase;
+
 public class CollisionHookTest extends TestCase
 {
     public void testCollisionHookInstall() throws Exception
@@ -45,6 +48,7 @@
 
         CollisionHook testCollisionHook = new CollisionHook()
         {
+            @Override
             public void filterCollisions(int operationType, Bundle target, Collection<Bundle> collisionCandidates)
             {
                 if ((target.getBundleId() == 4L) && (operationType == CollisionHook.INSTALLING))
@@ -60,7 +64,9 @@
         // Mock the framework
         StatefulResolver mockResolver = Mockito.mock(StatefulResolver.class);
         Felix felixMock = Mockito.mock(Felix.class);
-        Mockito.when(felixMock.getHooks(CollisionHook.class)).thenReturn(Collections.singleton(chRef));
+        HookRegistry hReg = mock(HookRegistry.class);
+        when(hReg.getHooks(CollisionHook.class)).thenReturn(Collections.singleton(chRef));
+        when(felixMock.getHookRegistry()).thenReturn(hReg);
         Mockito.when(felixMock.getResolver()).thenReturn(mockResolver);
         Mockito.when(felixMock.getBundles()).thenReturn(new Bundle[]
         {
@@ -104,6 +110,7 @@
 
         CollisionHook testCollisionHook = new CollisionHook()
         {
+            @Override
             public void filterCollisions(int operationType, Bundle target, Collection<Bundle> collisionCandidates)
             {
                 if ((target.getBundleId() == 3L) && (operationType == CollisionHook.UPDATING))
@@ -123,7 +130,9 @@
         StatefulResolver mockResolver = Mockito.mock(StatefulResolver.class);
         Felix felixMock = Mockito.mock(Felix.class);
         Mockito.when(felixMock.getConfig()).thenReturn(config);
-        Mockito.when(felixMock.getHooks(CollisionHook.class)).thenReturn(Collections.singleton(chRef));
+        HookRegistry hReg = mock(HookRegistry.class);
+        when(hReg.getHooks(CollisionHook.class)).thenReturn(Collections.singleton(chRef));
+        when(felixMock.getHookRegistry()).thenReturn(hReg);
         Mockito.when(felixMock.getResolver()).thenReturn(mockResolver);
         Mockito.when(felixMock.getBundles()).thenReturn(new Bundle[]
         {
@@ -160,6 +169,7 @@
 
         CollisionHook testCollisionHook = new CollisionHook()
         {
+            @Override
             public void filterCollisions(int operationType, Bundle target, Collection<Bundle> collisionCandidates)
             {
                 if ((target.getBundleId() == 3L) && (operationType == CollisionHook.INSTALLING))
@@ -179,7 +189,9 @@
         StatefulResolver mockResolver = Mockito.mock(StatefulResolver.class);
         Felix felixMock = Mockito.mock(Felix.class);
         Mockito.when(felixMock.getConfig()).thenReturn(config);
-        Mockito.when(felixMock.getHooks(CollisionHook.class)).thenReturn(Collections.singleton(chRef));
+        HookRegistry hReg = mock(HookRegistry.class);
+        when(hReg.getHooks(CollisionHook.class)).thenReturn(Collections.singleton(chRef));
+        when(felixMock.getHookRegistry()).thenReturn(hReg);
         Mockito.when(felixMock.getResolver()).thenReturn(mockResolver);
         Mockito.when(felixMock.getBundles()).thenReturn(new Bundle[]
         {
@@ -255,6 +267,8 @@
         // Mock the framework
         StatefulResolver mockResolver = Mockito.mock(StatefulResolver.class);
         Felix felixMock = Mockito.mock(Felix.class);
+        HookRegistry hReg = Mockito.mock(HookRegistry.class);
+        Mockito.when(felixMock.getHookRegistry()).thenReturn(hReg);
         Mockito.when(felixMock.getResolver()).thenReturn(mockResolver);
         Mockito.when(felixMock.getBundles()).thenReturn(new Bundle[]
         {