You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by dj...@apache.org on 2012/08/09 18:18:04 UTC

svn commit: r1371284 - in /felix/trunk/scr/src/main/java/org/apache/felix/scr/impl: config/ manager/

Author: djencks
Date: Thu Aug  9 16:18:03 2012
New Revision: 1371284

URL: http://svn.apache.org/viewvc?rev=1371284&view=rev
Log:
FELIX-3456 read-write lock for create impl object, atomic CAS for service registration.  Incomplete: no ServiceFactory or (possibly) ComponentFactory

Modified:
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/config/ImmediateComponentHolder.java Thu Aug  9 16:18:03 2012
@@ -26,7 +26,6 @@ import java.util.Map;
 
 import org.apache.felix.scr.Component;
 import org.apache.felix.scr.impl.BundleComponentActivator;
-import org.apache.felix.scr.impl.manager.ComponentFactoryImpl;
 import org.apache.felix.scr.impl.manager.DelayedComponentManager;
 import org.apache.felix.scr.impl.manager.ImmediateComponentManager;
 import org.apache.felix.scr.impl.manager.ServiceFactoryComponentManager;
@@ -189,14 +188,14 @@ public class ImmediateComponentHolder im
         if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
         {
             // singleton configuration deleted
-            m_singleComponent.obtainStateLock();
+            m_singleComponent.obtainReadLock();
             try
             {
                 m_singleComponent.reconfigure( null );
             }
             finally
             {
-                m_singleComponent.releaseStateLock();
+                m_singleComponent.releaseReadLock();
             }
         }
         else
@@ -206,7 +205,7 @@ public class ImmediateComponentHolder im
             if ( icm != null )
             {
                 boolean dispose = true;
-                icm.obtainStateLock();
+                icm.obtainReadLock();
                 try
                 {
                     // special casing if the single component is deconfigured
@@ -238,12 +237,12 @@ public class ImmediateComponentHolder im
                     // is not the "last" and has to be disposed off
                     if ( dispose )
                     {
-                        icm.dispose( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
+                        icm.disposeInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
                     }
                 }
                 finally
                 {
-                    icm.releaseStateLock();
+                    icm.releaseReadLock();
                 }
             }
         }
@@ -272,7 +271,7 @@ public class ImmediateComponentHolder im
 
         if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
         {
-            m_singleComponent.obtainStateLock();
+            m_singleComponent.obtainReadLock();
             try
             {
 // singleton configuration has pid equal to component name
@@ -280,7 +279,7 @@ public class ImmediateComponentHolder im
             }
             finally
             {
-                m_singleComponent.releaseStateLock();
+                m_singleComponent.releaseReadLock();
             }
         }
         else
@@ -289,7 +288,7 @@ public class ImmediateComponentHolder im
             final ImmediateComponentManager icm = getComponentManager( pid );
             if ( icm != null )
             {
-                icm.obtainStateLock();
+                icm.obtainReadLock();
                 try
                 {
                     // factory configuration updated for existing component instance
@@ -297,7 +296,7 @@ public class ImmediateComponentHolder im
                 }
                 finally
                 {
-                    icm.releaseStateLock();
+                    icm.releaseReadLock();
                 }
             }
             else
@@ -316,14 +315,14 @@ public class ImmediateComponentHolder im
                 }
 
                 // configure the component
-                newIcm.obtainStateLock();
+                newIcm.obtainReadLock();
                 try
                 {
                     newIcm.reconfigure( props );
                 }
                 finally
                 {
-                    newIcm.releaseStateLock();
+                    newIcm.releaseReadLock();
                 }
 
                 // enable the component if it is initially enabled

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/AbstractComponentManager.java Thu Aug  9 16:18:03 2012
@@ -28,7 +28,8 @@ import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.locks.ReentrantLock;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
 
 import org.apache.felix.scr.Component;
 import org.apache.felix.scr.Reference;
@@ -89,7 +90,7 @@ public abstract class AbstractComponentM
     private BundleComponentActivator m_activator;
 
     // The ServiceRegistration
-    private volatile ServiceRegistration m_serviceRegistration;
+    private final AtomicReferenceWrapper m_serviceRegistration = new AtomicReferenceWrapper(  );
 
     private final LockWrapper m_stateLock;
 
@@ -148,11 +149,12 @@ public abstract class AbstractComponentM
     }
 
     //ImmediateComponentHolder should be in this manager package and this should be default access.
-    public final void obtainStateLock()
+    public final void obtainReadLock()
     {
+//        new Exception("Stack trace obtainReadLock").printStackTrace();
         try
         {
-            if (!m_stateLock.tryLock(  m_timeout ) )
+            if (!m_stateLock.tryReadLock( m_timeout ) )
             {
                 throw new IllegalStateException( "Could not obtain lock" );
             }
@@ -165,20 +167,49 @@ public abstract class AbstractComponentM
     }
 
 
-    public final void releaseStateLock()
+    public final void releaseReadLock()
     {
-        m_stateLock.unlock();
+//        new Exception("Stack trace releaseReadLock").printStackTrace();
+        m_stateLock.unlockReadLock();
     }
 
+    public final void escalateLock()
+    {
+//        new Exception("Stack trace escalateLock").printStackTrace();
+        m_stateLock.unlockReadLock();
+        try
+        {
+            if (!m_stateLock.tryWriteLock( m_timeout ) )
+            {
+                throw new IllegalStateException( "Could not obtain lock" );
+            }
+        }
+        catch ( InterruptedException e )
+        {
+            //TODO this is so wrong
+            throw new IllegalStateException( "Could not obtain lock (Reason: " + e + ")" );
+        }
+    }
+
+    public final void deescalateLock()
+    {
+//        new Exception("Stack trace deescalateLock").printStackTrace();
+        m_stateLock.deescalate();
+    }
 
     public final void checkLocked()
     {
-        if ( m_stateLock.getHoldCount() == 0 )
+        if ( m_stateLock.getReadHoldCount() == 0 && m_stateLock.getWriteHoldCount() == 0 )
         {
             throw new IllegalStateException( "State lock should be held by current thread" );
         }
     }
 
+    public final boolean isWriteLocked()
+    {
+        return m_stateLock.getWriteHoldCount() > 0;
+    }
+
 //---------- Component ID management
 
     void registerComponentId()
@@ -225,7 +256,7 @@ public abstract class AbstractComponentM
 
     public final void enable( final boolean async )
     {
-        obtainStateLock();
+        obtainReadLock();
         try
         {
             enableInternal();
@@ -236,7 +267,7 @@ public abstract class AbstractComponentM
         }
         finally
         {
-            releaseStateLock();
+            releaseReadLock();
         }
 
         if ( async )
@@ -245,14 +276,14 @@ public abstract class AbstractComponentM
             {
                 public void run()
                 {
-                    obtainStateLock();
+                    obtainReadLock();
                     try
                     {
                         activateInternal();
                     }
                     finally
                     {
-                        releaseStateLock();
+                        releaseReadLock();
                     }
                 }
             } );
@@ -273,7 +304,7 @@ public abstract class AbstractComponentM
 
     public final void disable( final boolean async )
     {
-        obtainStateLock();
+        obtainReadLock();
         try
         {
             if ( !async )
@@ -284,7 +315,7 @@ public abstract class AbstractComponentM
         }
         finally
         {
-            releaseStateLock();
+            releaseReadLock();
         }
 
         if ( async )
@@ -293,14 +324,14 @@ public abstract class AbstractComponentM
             {
                 public void run()
                 {
-                    obtainStateLock();
+                    obtainReadLock();
                     try
                     {
                         deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED );
                     }
                     finally
                     {
-                        releaseStateLock();
+                        releaseReadLock();
                     }
                 }
             } );
@@ -331,14 +362,14 @@ public abstract class AbstractComponentM
      */
     public void dispose( int reason )
     {
-        obtainStateLock();
+        obtainReadLock();
         try
         {
             disposeInternal( reason );
         }
         finally
         {
-            releaseStateLock();
+            releaseReadLock();
         }
     }
 
@@ -502,7 +533,7 @@ public abstract class AbstractComponentM
      * method has to actually complete before other actions like bundle stopping
      * may continue.
      */
-    final void disposeInternal( int reason )
+    public final void disposeInternal( int reason )
     {
         m_state.dispose( this );
     }
@@ -566,21 +597,35 @@ public abstract class AbstractComponentM
      * @return The <code>ServiceRegistration</code> for the registered
      *      service or <code>null</code> if no service is registered.
      */
-    protected ServiceRegistration registerService()
+    protected void registerService()
     {
         if ( getComponentMetadata().getServiceMetadata() != null )
         {
+            String[] provides = getComponentMetadata().getServiceMetadata().getProvides();
+            registerService( provides );
+        }
+    }
+
+    protected void registerService( String[] provides )
+    {
+            ServiceRegistration existing = m_serviceRegistration.get();
+        if ( existing == null )
+        {
             log( LogService.LOG_DEBUG, "registering services", null );
 
             // get a copy of the component properties as service properties
             final Dictionary serviceProperties = getServiceProperties();
 
-            return getActivator().getBundleContext().registerService(
-                    getComponentMetadata().getServiceMetadata().getProvides(),
-                    getService(), serviceProperties );
+            ServiceRegistration newRegistration = getActivator().getBundleContext().registerService(
+                provides,
+                getService(), serviceProperties );
+            boolean weWon = m_serviceRegistration.compareAndSet( existing, newRegistration );
+            if (weWon)
+            {
+                return;
+            }
+            newRegistration.unregister();
         }
-
-        return null;
     }
 
     /**
@@ -593,20 +638,14 @@ public abstract class AbstractComponentM
      */
     final void registerComponentService()
     {
-
-        if (this.m_serviceRegistration != null)
-        {
-            throw new IllegalStateException( "Component service already registered: " + this );
-        }
-        this.m_serviceRegistration = registerService();
+        registerService();
     }
 
     final void unregisterComponentService()
     {
-        ServiceRegistration sr = this.m_serviceRegistration;
-        this.m_serviceRegistration = null;
+        ServiceRegistration sr = m_serviceRegistration.get();
 
-        if ( sr != null )
+        if ( sr != null && m_serviceRegistration.compareAndSet( sr, null ) )
         {
             log( LogService.LOG_DEBUG, "Unregistering the services", null );
             sr.unregister();
@@ -630,7 +669,7 @@ public abstract class AbstractComponentM
 
     final ServiceRegistration getServiceRegistration()
     {
-        return m_serviceRegistration;
+        return m_serviceRegistration.get();
     }
 
 
@@ -1028,7 +1067,7 @@ public abstract class AbstractComponentM
         }
 
 
-        void ungetService( DelayedComponentManager dcm )
+        void ungetService( ImmediateComponentManager dcm )
         {
 //            log( dcm, "ungetService" );
             throw new IllegalStateException("ungetService" + this);
@@ -1081,8 +1120,16 @@ public abstract class AbstractComponentM
             try
             {
                 acm.unregisterComponentService();
-                acm.deleteComponent( reason );
-                acm.deactivateDependencyManagers();
+                acm.escalateLock();
+                try
+                {
+                    acm.deleteComponent( reason );
+                    acm.deactivateDependencyManagers();
+                }
+                finally
+                {
+                    acm.deescalateLock();
+                }
             }
             catch ( Throwable t )
             {
@@ -1194,7 +1241,7 @@ public abstract class AbstractComponentM
                 return;
             }
 
-            // set satisifed state before registering the service because
+            // set satisfied state before registering the service because
             // during service registration a listener may try to get the
             // service from the service reference which may cause a
             // delayed service object instantiation through the State
@@ -1210,7 +1257,7 @@ public abstract class AbstractComponentM
             // test if all the mandatory dependencies are satisfied
             if ( !acm.verifyDependencyManagers( acm.getProperties() ) )
             {
-                acm.log( LogService.LOG_DEBUG, "Not all dependencies satisified, cannot activate", null );
+                acm.log( LogService.LOG_DEBUG, "Not all dependencies satisfied, cannot activate", null );
                 acm.changeState( Unsatisfied.getInstance() );
                 return;
             }
@@ -1231,13 +1278,23 @@ public abstract class AbstractComponentM
             // 2. Create the component instance and component context
             // 3. Bind the target services
             // 4. Call the activate method, if present
-            if ( ( acm.isImmediate() || acm.getComponentMetadata().isFactory() ) && !acm.createComponent() )
+            if ( ( acm.isImmediate() || acm.getComponentMetadata().isFactory() ) )
             {
-                // component creation failed, not active now
-                acm.log( LogService.LOG_ERROR, "Component instance could not be created, activation failed", null );
+                acm.escalateLock();
+                try
+                {
+                    if ( !acm.createComponent() )
+                    {
+                        // component creation failed, not active now
+                        acm.log( LogService.LOG_ERROR, "Component instance could not be created, activation failed", null );
+                        acm.changeState( Unsatisfied.getInstance() );
+                    }
+                }
+                finally
+                {
+                    acm.deescalateLock();
+                }
 
-                // set state to unsatisfied
-                acm.changeState( Unsatisfied.getInstance() );
             }
 
         }
@@ -1335,7 +1392,7 @@ public abstract class AbstractComponentM
         }
 
 
-        void ungetService( DelayedComponentManager dcm )
+        void ungetService( ImmediateComponentManager dcm )
         {
             dcm.deleteComponent( ComponentConstants.DEACTIVATION_REASON_UNSPECIFIED );
             dcm.changeState( Registered.getInstance() );
@@ -1447,7 +1504,7 @@ public abstract class AbstractComponentM
         void deactivate( AbstractComponentManager acm, int reason )
         {
             acm.changeState( Active.getInstance() );
-            acm.dispose( reason );
+            acm.disposeInternal( reason );
         }
     }
 
@@ -1498,28 +1555,56 @@ public abstract class AbstractComponentM
 
     private static interface LockWrapper
     {
-        boolean tryLock( long milliseconds ) throws InterruptedException;
-        long getHoldCount();
-        void unlock();
+        boolean tryReadLock( long milliseconds ) throws InterruptedException;
+        long getReadHoldCount();
+        void unlockReadLock();
+        
+        boolean tryWriteLock( long milliseconds ) throws InterruptedException;
+        long getWriteHoldCount();
+        void unlockWriteLock();
+        void deescalate();
+
+        
     }
 
     private static class JLock implements LockWrapper
     {
-        private final ReentrantLock lock = new ReentrantLock( true );
+        private final ReentrantReadWriteLock lock = new ReentrantReadWriteLock( true );
+
+        public boolean tryReadLock( long milliseconds ) throws InterruptedException
+        {
+             return lock.readLock().tryLock( milliseconds, TimeUnit.MILLISECONDS );
+        }
+
+        public long getReadHoldCount()
+        {
+            return lock.getReadHoldCount();
+        }
+
+        public void unlockReadLock()
+        {
+            lock.readLock().unlock();
+        }
 
-        public boolean tryLock( long milliseconds ) throws InterruptedException
+        public boolean tryWriteLock( long milliseconds ) throws InterruptedException
         {
-             return lock.tryLock( milliseconds, TimeUnit.MILLISECONDS );
+            return lock.writeLock().tryLock( milliseconds, TimeUnit.MILLISECONDS );
         }
 
-        public long getHoldCount()
+        public long getWriteHoldCount()
         {
-            return lock.getHoldCount();
+            return lock.getWriteHoldCount();
         }
 
-        public void unlock()
+        public void unlockWriteLock()
         {
-            lock.unlock();
+            lock.writeLock().unlock();
+        }
+
+        public void deescalate()
+        {
+            lock.readLock().lock();
+            lock.writeLock().unlock();
         }
     }
 
@@ -1527,19 +1612,52 @@ public abstract class AbstractComponentM
     {
         private final EDU.oswego.cs.dl.util.concurrent.ReentrantLock lock = new EDU.oswego.cs.dl.util.concurrent.ReentrantLock();
 
-        public boolean tryLock( long milliseconds ) throws InterruptedException
+        public boolean tryReadLock( long milliseconds ) throws InterruptedException
         {
             return lock.attempt( milliseconds );
         }
 
-        public long getHoldCount()
+        public long getReadHoldCount()
         {
             return lock.holds();
         }
 
-        public void unlock()
+        public void unlockReadLock()
         {
             lock.release();
         }
+
+        public boolean tryWriteLock( long milliseconds ) throws InterruptedException
+        {
+            return false;
+        }
+
+        public long getWriteHoldCount()
+        {
+            return 0;
+        }
+
+        public void unlockWriteLock()
+        {
+        }
+
+        public void deescalate()
+        {
+        }
+    }
+
+    static class AtomicReferenceWrapper
+    {
+        private final AtomicReference ref = new AtomicReference(  );
+
+        public ServiceRegistration get()
+        {
+            return ( ServiceRegistration ) ref.get();
+        }
+
+        public boolean compareAndSet(ServiceRegistration expected, ServiceRegistration replacement)
+        {
+            return ref.compareAndSet( expected, replacement );
+        }
     }
 }

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentFactoryImpl.java Thu Aug  9 16:18:03 2012
@@ -94,7 +94,7 @@ public class ComponentFactoryImpl extend
         final ImmediateComponentManager cm = createComponentManager();
 
         ComponentInstance instance;
-        cm.obtainStateLock();
+        cm.obtainReadLock();
         try
         {
             cm.setFactoryProperties( dictionary );
@@ -114,7 +114,7 @@ public class ComponentFactoryImpl extend
         }
         finally
         {
-            cm.releaseStateLock();
+            cm.releaseReadLock();
         }
 
         synchronized ( m_componentInstances )
@@ -179,13 +179,11 @@ public class ComponentFactoryImpl extend
     }
 
 
-    protected ServiceRegistration registerService()
+    protected void registerService()
     {
         log( LogService.LOG_DEBUG, "registering component factory", null );
-
-        Dictionary serviceProperties = getServiceProperties();
-        return getActivator().getBundleContext().registerService( new String[]
-            { ComponentFactory.class.getName() }, getService(), serviceProperties );
+        registerService(new String[]
+            { ComponentFactory.class.getName() });
     }
 
 

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ConfigurationComponentFactoryImpl.java Thu Aug  9 16:18:03 2012
@@ -139,7 +139,7 @@ public class ConfigurationComponentFacto
         }
         else   //non-spec backwards compatible
         {
-            obtainStateLock();
+            obtainReadLock();
             try
             {
                 ImmediateComponentManager cm;
@@ -178,20 +178,20 @@ public class ConfigurationComponentFacto
                 {
                     // update the configuration as if called as ManagedService
                     //TODO deadlock potential, we are holding our own state lock.
-                    cm.obtainStateLock();
+                    cm.obtainReadLock();
                     try
                     {
                         cm.reconfigure( configuration );
                     }
                     finally
                     {
-                        cm.releaseStateLock();
+                        cm.releaseReadLock();
                     }
                 }
             }
             finally
             {
-                releaseStateLock();
+                releaseReadLock();
             }
         }
     }

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DelayedComponentManager.java Thu Aug  9 16:18:03 2012
@@ -23,7 +23,6 @@ import org.apache.felix.scr.impl.BundleC
 import org.apache.felix.scr.impl.config.ComponentHolder;
 import org.apache.felix.scr.impl.metadata.ComponentMetadata;
 import org.osgi.framework.Bundle;
-import org.osgi.framework.ServiceFactory;
 import org.osgi.framework.ServiceRegistration;
 
 
@@ -33,8 +32,8 @@ import org.osgi.framework.ServiceRegistr
 public class DelayedComponentManager extends ImmediateComponentManager
 {
 
-    // keep the using bundles as reference "counters" for instance deactivation
-    private int m_useCount;
+//    // keep the using bundles as reference "counters" for instance deactivation
+//    private int m_useCount;
 
 
     /**
@@ -45,20 +44,20 @@ public class DelayedComponentManager ext
         ComponentMetadata metadata )
     {
         super( activator, componentHolder, metadata );
-        this.m_useCount = 0;
+//        this.m_useCount = 0;
     }
 
-    protected void deleteComponent( int reason )
-    {
-        // only have to delete, if there is actually an instance
-        if ( getInstance() != null )
-        {
-            super.deleteComponent( reason );
-        }
-
-        // ensure the refence set is also clear
-        m_useCount = 0;
-    }
+//    protected void deleteComponent( int reason )
+//    {
+//        // only have to delete, if there is actually an instance
+//        if ( getInstance() != null )
+//        {
+//            super.deleteComponent( reason );
+//        }
+//
+//        // ensure the refence set is also clear
+////        m_useCount = 0;
+//    }
 
     State getSatisfiedState()
     {
@@ -67,43 +66,43 @@ public class DelayedComponentManager ext
 
     //---------- ServiceFactory interface -------------------------------------
 
-    public Object getService( Bundle bundle, ServiceRegistration sr )
-    {
-        obtainStateLock();
-        try
-        {
-            m_useCount++;
-            return state().getService( this );
-        }
-        finally
-        {
-            releaseStateLock();
-        }
-    }
-
-    public void ungetService( Bundle bundle, ServiceRegistration sr, Object service )
-    {
-        obtainStateLock();
-        try
-        {
-            // the framework should not call ungetService more than it calls
-            // calls getService. Still, we want to be sure to not go below zero
-            if ( m_useCount > 0 )
-            {
-                m_useCount--;
-
-                // unget the service instance if no bundle is using it
-                // any longer unless delayed component instances have to
-                // be kept (FELIX-3039)
-                if ( m_useCount == 0 && !getActivator().getConfiguration().keepInstances() )
-                {
-                    state().ungetService( this );
-                }
-            }
-        }
-        finally
-        {
-            releaseStateLock();
-        }
-    }
+//    public Object getService( Bundle bundle, ServiceRegistration sr )
+//    {
+//        obtainReadLock();
+//        try
+//        {
+//            m_useCount++;
+//            return state().getService( this );
+//        }
+//        finally
+//        {
+//            releaseReadLock();
+//        }
+//    }
+//
+//    public void ungetService( Bundle bundle, ServiceRegistration sr, Object service )
+//    {
+//        obtainReadLock();
+//        try
+//        {
+//            // the framework should not call ungetService more than it calls
+//            // calls getService. Still, we want to be sure to not go below zero
+//            if ( m_useCount > 0 )
+//            {
+//                m_useCount--;
+//
+//                // unget the service instance if no bundle is using it
+//                // any longer unless delayed component instances have to
+//                // be kept (FELIX-3039)
+//                if ( m_useCount == 0 && !getActivator().getConfiguration().keepInstances() )
+//                {
+//                    state().ungetService( this );
+//                }
+//            }
+//        }
+//        finally
+//        {
+//            releaseReadLock();
+//        }
+//    }
 }

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java Thu Aug  9 16:18:03 2012
@@ -71,7 +71,7 @@ public class DependencyManager implement
     private final Map m_bound;
 
     // the number of matching services registered in the system
-    private int m_size;
+    private volatile int m_size;
 
     // the object on which the bind/undind methods are to be called
     private volatile Object m_componentInstance;
@@ -155,7 +155,7 @@ public class DependencyManager implement
         final ServiceReference ref = event.getServiceReference();
         final String serviceString = "Service " + m_dependencyMetadata.getInterface() + "/"
             + ref.getProperty( Constants.SERVICE_ID );
-        m_componentManager.obtainStateLock();
+        m_componentManager.obtainReadLock();
         try
         {
             switch ( event.getType() )
@@ -258,7 +258,7 @@ public class DependencyManager implement
         }
         finally
         {
-            m_componentManager.releaseStateLock();
+            m_componentManager.releaseReadLock();
         }
     }
 

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java Thu Aug  9 16:18:03 2012
@@ -52,6 +52,9 @@ public class ImmediateComponentManager e
     // The object that implements the service and that is bound to other services
     private Object m_implementationObject;
 
+    // keep the using bundles as reference "counters" for instance deactivation
+    private volatile int m_useCount;
+
     // The context that will be passed to the implementationObject
     private ComponentContextImpl m_componentContext;
 
@@ -89,7 +92,7 @@ public class ImmediateComponentManager e
      * @param metadata
      */
     public ImmediateComponentManager( BundleComponentActivator activator, ComponentHolder componentHolder,
-        ComponentMetadata metadata )
+            ComponentMetadata metadata )
     {
         super( activator, metadata );
 
@@ -115,6 +118,10 @@ public class ImmediateComponentManager e
     // also be overwritten
     protected boolean createComponent()
     {
+        if ( !isWriteLocked() )
+        {
+            throw new IllegalStateException( "need write lock (createComponent)" );
+        }
         if ( m_implementationObject == null )
         {
             ComponentContextImpl tmpContext = new ComponentContextImpl( this );
@@ -136,11 +143,18 @@ public class ImmediateComponentManager e
 
     protected void deleteComponent( int reason )
     {
-        disposeImplementationObject( m_implementationObject, m_componentContext, reason );
-        m_implementationObject = null;
-        m_componentContext = null;
-        m_properties = null;
-        m_serviceProperties = null;
+        if ( !isWriteLocked() )
+        {
+            throw new IllegalStateException( "need write lock (deleteComponent)" );
+        }
+        if ( m_implementationObject != null )
+        {
+            disposeImplementationObject( m_implementationObject, m_componentContext, reason );
+            m_implementationObject = null;
+            m_componentContext = null;
+            m_properties = null;
+            m_serviceProperties = null;
+        }
     }
 
 
@@ -153,10 +167,10 @@ public class ImmediateComponentManager e
     //**********************************************************************************************************
 
     /**
-    * Get the object that is implementing this descriptor
-    *
-    * @return the object that implements the services
-    */
+     * Get the object that is implementing this descriptor
+     *
+     * @return the object that implements the services
+     */
     Object getInstance()
     {
         return m_implementationObject;
@@ -175,7 +189,7 @@ public class ImmediateComponentManager e
         {
             // 112.4.4 The class is retrieved with the loadClass method of the component's bundle
             implementationObjectClass = getActivator().getBundleContext().getBundle().loadClass(
-                getComponentMetadata().getImplementationClassName() );
+                    getComponentMetadata().getImplementationClassName() );
 
             // 112.4.4 The class must be public and have a public constructor without arguments so component instances
             // may be created by the SCR with the newInstance method on Class
@@ -199,8 +213,8 @@ public class ImmediateComponentManager e
             if ( !dm.open( implementationObject ) )
             {
                 log( LogService.LOG_ERROR, "Cannot create component instance due to failure to bind reference {0}",
-                    new Object[]
-                        { dm.getName() }, null );
+                        new Object[]
+                                {dm.getName()}, null );
 
                 // make sure, we keep no bindings
                 it = getReversedDependencyManagers();
@@ -215,15 +229,15 @@ public class ImmediateComponentManager e
         }
 
         // get the method
-        if ( m_activateMethod == null)
+        if ( m_activateMethod == null )
         {
             m_activateMethod = new ActivateMethod( this, getComponentMetadata().getActivate(), getComponentMetadata()
-                .isActivateDeclared(), implementationObjectClass );
+                    .isActivateDeclared(), implementationObjectClass );
         }
 
         // 4. Call the activate method, if present
-        final MethodResult result = m_activateMethod.invoke(implementationObject, new ActivatorParameter(
-            componentContext, 1), null);
+        final MethodResult result = m_activateMethod.invoke( implementationObject, new ActivatorParameter(
+                componentContext, 1 ), null );
         if ( result == null )
         {
             // 112.5.8 If the activate method throws an exception, SCR must log an error message
@@ -247,14 +261,14 @@ public class ImmediateComponentManager e
 
 
     protected void disposeImplementationObject( Object implementationObject, ComponentContext componentContext,
-        int reason )
+            int reason )
     {
 
         // get the method
         if ( m_deactivateMethod == null )
         {
             m_deactivateMethod = new DeactivateMethod( this, getComponentMetadata().getDeactivate(),
-                getComponentMetadata().isDeactivateDeclared(), implementationObject.getClass() );
+                    getComponentMetadata().isDeactivateDeclared(), implementationObject.getClass() );
         }
 
         // 1. Call the deactivate method, if present
@@ -262,7 +276,7 @@ public class ImmediateComponentManager e
         // method throws an exception, SCR must log an error message containing the
         // exception with the Log Service and continue) has already been logged
         final MethodResult result = m_deactivateMethod.invoke( implementationObject, new ActivatorParameter( componentContext,
-            reason ), null );
+                reason ), null );
         if ( result != null )
         {
             setServiceProperties( result );
@@ -284,7 +298,7 @@ public class ImmediateComponentManager e
     /**
      * Returns the service object to be registered if the service element is
      * specified.
-     * <p>
+     * <p/>
      * Extensions of this class may overwrite this method to return a
      * ServiceFactory to register in the case of a delayed or a service
      * factory component.
@@ -328,7 +342,7 @@ public class ImmediateComponentManager e
     /**
      * Returns the (private copy) of the Component properties to be used
      * for the ComponentContext as well as eventual service registration.
-     * <p>
+     * <p/>
      * Method implements the Component Properties provisioning as described
      * in 112.6, Component Properties.
      *
@@ -379,7 +393,7 @@ public class ImmediateComponentManager e
         }
         else
         {
-            m_serviceProperties = copyTo(null, serviceProperties, false);
+            m_serviceProperties = copyTo( null, serviceProperties, false );
             // set component.name and component.id
             m_serviceProperties.put( ComponentConstants.COMPONENT_NAME, getComponentMetadata().getName() );
             m_serviceProperties.put( ComponentConstants.COMPONENT_ID, new Long( getId() ) );
@@ -388,7 +402,8 @@ public class ImmediateComponentManager e
         updateServiceRegistration();
     }
 
-    public Dictionary getServiceProperties() {
+    public Dictionary getServiceProperties()
+    {
         if ( m_serviceProperties != null )
         {
             return m_serviceProperties;
@@ -399,29 +414,29 @@ public class ImmediateComponentManager e
     private void updateServiceRegistration()
     {
         ServiceRegistration sr = getServiceRegistration();
-        if (sr != null)
+        if ( sr != null )
         {
             try
             {
                 // Don't propagate if service properties did not change.
                 final Dictionary regProps = getServiceProperties();
-                if (!servicePropertiesMatches(sr, regProps))
+                if ( !servicePropertiesMatches( sr, regProps ) )
                 {
-                    sr.setProperties(regProps);
+                    sr.setProperties( regProps );
                 }
             }
-            catch (IllegalStateException ise)
+            catch ( IllegalStateException ise )
             {
                 // service has been unregistered asynchronously, ignore
             }
-            catch (IllegalArgumentException iae)
+            catch ( IllegalArgumentException iae )
             {
-                log(LogService.LOG_ERROR,
-                    "Unexpected configuration property problem when updating service registration", iae);
+                log( LogService.LOG_ERROR,
+                        "Unexpected configuration property problem when updating service registration", iae );
             }
-            catch (Throwable t)
+            catch ( Throwable t )
             {
-                log(LogService.LOG_ERROR, "Unexpected problem when updating service registration", t);
+                log( LogService.LOG_ERROR, "Unexpected problem when updating service registration", t );
             }
         }
     }
@@ -429,7 +444,7 @@ public class ImmediateComponentManager e
     /**
      * Called by the Configuration Admin Service to update the component with
      * Configuration properties.
-     * <p>
+     * <p/>
      * This causes the component to be reactivated with the new configuration
      * unless no configuration has ever been set on this component and the
      * <code>configuration</code> parameter is <code>null</code>. In this case
@@ -439,8 +454,8 @@ public class ImmediateComponentManager e
      * with the default configuration.
      *
      * @param configuration The configuration properties for the component from
-     *      the Configuration Admin Service or <code>null</code> if there is
-     *      no configuration or if the configuration has just been deleted.
+     *                      the Configuration Admin Service or <code>null</code> if there is
+     *                      no configuration or if the configuration has just been deleted.
      */
     public void reconfigure( Dictionary configuration )
     {
@@ -460,7 +475,7 @@ public class ImmediateComponentManager e
         // unsatisfied component and non-ignored configuration may change targets
         // to satisfy references
         if ( getState() == STATE_UNSATISFIED && configuration != null
-            && !getComponentMetadata().isConfigurationIgnored() )
+                && !getComponentMetadata().isConfigurationIgnored() )
         {
             activateInternal();
             return;
@@ -485,7 +500,7 @@ public class ImmediateComponentManager e
             // SCR 112.7.1 - deactivate if configuration is deleted or no modified method declared
             log( LogService.LOG_DEBUG, "Deactivating and Activating to reconfigure from configuration", null );
             int reason = ( configuration == null ) ? ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED
-                : ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_MODIFIED;
+                    : ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_MODIFIED;
 
             // FELIX-2368: cycle component immediately, reconfigure() is
             //     called through ConfigurationListener API which itself is
@@ -495,7 +510,8 @@ public class ImmediateComponentManager e
         }
     }
 
-    private boolean modify() {
+    private boolean modify()
+    {
         // 0. no live update if there is no instance
         if ( getInstance() == null )
         {
@@ -526,23 +542,23 @@ public class ImmediateComponentManager e
             if ( !dm.canUpdateDynamically( props ) )
             {
                 log( LogService.LOG_DEBUG,
-                    "Cannot dynamically update the configuration due to dependency changes induced on dependency {0}",
-                    new Object[]
-                        { dm.getName() }, null );
+                        "Cannot dynamically update the configuration due to dependency changes induced on dependency {0}",
+                        new Object[]
+                                {dm.getName()}, null );
                 return false;
             }
         }
         // invariant: modify method existing and no static bound service changes
 
         // 4. call method (nothing to do when failed, since it has already been logged)
-        final MethodResult result = m_modifyMethod.invoke(getInstance(),
-            new ActivatorParameter(m_componentContext, -1), null);
+        final MethodResult result = m_modifyMethod.invoke( getInstance(),
+                new ActivatorParameter( m_componentContext, -1 ), null );
         if ( result == null )
         {
             // log an error if the declared method cannot be found
             log( LogService.LOG_ERROR, "Declared modify method ''{0}'' cannot be found, configuring by reactivation",
-                new Object[]
-                    { getComponentMetadata().getModified() }, null );
+                    new Object[]
+                            {getComponentMetadata().getModified()}, null );
             return false;
         }
 
@@ -552,9 +568,9 @@ public class ImmediateComponentManager e
         if ( !verifyDependencyManagers( props ) )
         {
             log(
-                LogService.LOG_ERROR,
-                "Updating the service references caused at least on reference to become unsatisfied, deactivating component",
-                null );
+                    LogService.LOG_ERROR,
+                    "Updating the service references caused at least on reference to become unsatisfied, deactivating component",
+                    null );
             return false;
         }
 
@@ -577,12 +593,12 @@ public class ImmediateComponentManager e
      * Checks if the given service registration properties matches another set
      * of properties.
      *
-     * @param reg the service registration whose service properties will be
-     *      compared to the props parameter
+     * @param reg   the service registration whose service properties will be
+     *              compared to the props parameter
      * @param props the properties to be compared with the registration
-     *      service properties.
+     *              service properties.
      * @return <code>true</code> if the registration service properties equals
-     *      the prop properties, false if not.
+     *         the prop properties, false if not.
      */
     private boolean servicePropertiesMatches( ServiceRegistration reg, Dictionary props )
     {
@@ -591,7 +607,7 @@ public class ImmediateComponentManager e
         for ( int i = 0; keys != null && i < keys.length; i++ )
         {
             if ( !keys[i].equals( org.osgi.framework.Constants.OBJECTCLASS )
-                && !keys[i].equals( org.osgi.framework.Constants.SERVICE_ID ) )
+                    && !keys[i].equals( org.osgi.framework.Constants.SERVICE_ID ) )
             {
                 regProps.put( keys[i], reg.getReference().getProperty( keys[i] ) );
             }
@@ -601,23 +617,68 @@ public class ImmediateComponentManager e
 
     public Object getService( Bundle bundle, ServiceRegistration serviceRegistration )
     {
-        obtainStateLock();
+        obtainReadLock();
         try
         {
-            if (m_implementationObject != null)
+            if ( m_useCount == 0 )
             {
-                return m_implementationObject;
+                escalateLock();
+                try
+                {
+                    if ( m_useCount == 0 )
+                    {
+                        m_useCount++;
+                        return state().getService( this );
+                    }
+                }
+                finally
+                {
+                    deescalateLock();
+                }
             }
-//            m_useCount++;
-            return Registered.getInstance().getService( this );
+            m_useCount++;
+            return state().getService( this );
         }
         finally
         {
-            releaseStateLock();
+            releaseReadLock();
         }
     }
 
     public void ungetService( Bundle bundle, ServiceRegistration serviceRegistration, Object o )
     {
+        obtainReadLock();
+        try
+        {
+            // the framework should not call ungetService more than it calls
+            // calls getService. Still, we want to be sure to not go below zero
+            if ( m_useCount > 0 )
+            {
+                m_useCount--;
+
+                // unget the service instance if no bundle is using it
+                // any longer unless delayed component instances have to
+                // be kept (FELIX-3039)
+                if ( m_useCount == 0 && !isImmediate() && !getActivator().getConfiguration().keepInstances() )
+                {
+                    escalateLock();
+                    try
+                    {
+                        if ( m_useCount == 0 )
+                        {
+                            state().ungetService( this );
+                        }
+                    }
+                    finally
+                    {
+                        deescalateLock();
+                    }
+                }
+            }
+        }
+        finally
+        {
+            releaseReadLock();
+        }
     }
 }

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java?rev=1371284&r1=1371283&r2=1371284&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ServiceFactoryComponentManager.java Thu Aug  9 16:18:03 2012
@@ -25,7 +25,6 @@ import org.apache.felix.scr.impl.BundleC
 import org.apache.felix.scr.impl.config.ComponentHolder;
 import org.apache.felix.scr.impl.metadata.ComponentMetadata;
 import org.osgi.framework.Bundle;
-import org.osgi.framework.ServiceFactory;
 import org.osgi.framework.ServiceRegistration;
 import org.osgi.service.component.ComponentConstants;
 import org.osgi.service.component.ComponentContext;
@@ -96,7 +95,7 @@ public class ServiceFactoryComponentMana
 
         // When the getServiceMethod is called, the implementation object must be created
 
-        obtainStateLock();
+        obtainReadLock();
         try
         {
 // private ComponentContext and implementation instances
@@ -127,7 +126,7 @@ public class ServiceFactoryComponentMana
         }
         finally
         {
-            releaseStateLock();
+            releaseReadLock();
         }
     }
 
@@ -140,7 +139,7 @@ public class ServiceFactoryComponentMana
         log( LogService.LOG_DEBUG, "ServiceFactory.ungetService()", null );
 
         // When the ungetServiceMethod is called, the implementation object must be deactivated
-        obtainStateLock();
+        obtainReadLock();
         try
         {
 // private ComponentContext and implementation instances
@@ -157,7 +156,7 @@ public class ServiceFactoryComponentMana
         }
         finally
         {
-            releaseStateLock();
+            releaseReadLock();
         }
     }