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/10/25 20:00:21 UTC

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

Author: djencks
Date: Thu Oct 25 18:00:20 2012
New Revision: 1402238

URL: http://svn.apache.org/viewvc?rev=1402238&view=rev
Log:
FELIX-3680 Intermediate commit which disables read lock, synchronizes service registration, moves service listener deregistration to deactivate thread (controlled by flag)

Modified:
    felix/trunk/scr/pom.xml
    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/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/pom.xml
URL: http://svn.apache.org/viewvc/felix/trunk/scr/pom.xml?rev=1402238&r1=1402237&r2=1402238&view=diff
==============================================================================
--- felix/trunk/scr/pom.xml (original)
+++ felix/trunk/scr/pom.xml Thu Oct 25 18:00:20 2012
@@ -307,8 +307,8 @@
                             kxml2;inline=org/kxml2/io/KXmlParser.class|org/xmlpull/v1/XmlPull**,
                             backport-util-concurrent;inline=edu/emory/mathcs/backport/java/util/concurrent/TimeUnit.class
                             |edu/emory/mathcs/backport/java/util/concurrent/TimeUnit*.class
-                            |edu/emory/mathcs/backport/java/util/concurrent/locks/ReentrantReadWriteLock.class
-                            |edu/emory/mathcs/backport/java/util/concurrent/locks/ReentrantReadWriteLock*.class
+                            |edu/emory/mathcs/backport/java/util/concurrent/locks/ReentrantLock.class
+                            |edu/emory/mathcs/backport/java/util/concurrent/locks/ReentrantLock*.class
                             |edu/emory/mathcs/backport/java/util/concurrent/locks/ReadWriteLock.class
                             |edu/emory/mathcs/backport/java/util/concurrent/locks/Lock.class
                             |edu/emory/mathcs/backport/java/util/concurrent/locks/Condition.class
@@ -343,9 +343,7 @@
                     </signature>
                     <ignores>
                         <ignore>java.util.concurrent.atomic.AtomicReference</ignore>
-                        <ignore>java.util.concurrent.locks.ReentrantReadWriteLock</ignore>
-                        <ignore>java.util.concurrent.locks.ReentrantReadWriteLock$ReadLock</ignore>
-                        <ignore>java.util.concurrent.locks.ReentrantReadWriteLock$WriteLock</ignore>
+                        <ignore>java.util.concurrent.locks.ReentrantLock</ignore>
                         <ignore>java.util.concurrent.TimeUnit</ignore>
                         <ignore>java.lang.management.ManagementFactory</ignore>
                         <ignore>java.lang.management.ThreadInfo</ignore>

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=1402238&r1=1402237&r2=1402238&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 Oct 25 18:00:20 2012
@@ -192,19 +192,7 @@ public class ImmediateComponentHolder im
 
         if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
         {
-            // singleton configuration deleted
-            final boolean release = m_singleComponent.obtainReadLock( "ImmediateComponentHolder.configurationDeleted.1" );
-            try
-            {
-                m_singleComponent.reconfigure( null );
-            }
-            finally
-            {
-                if ( release )
-                {
-                    m_singleComponent.releaseReadLock( "ImmediateComponentHolder.configurationDeleted.1" );
-                }
-            }
+            m_singleComponent.reconfigure( null );
         }
         else
         {
@@ -213,47 +201,36 @@ public class ImmediateComponentHolder im
             if ( icm != null )
             {
                 boolean dispose = true;
-                final boolean release = icm.obtainReadLock( "ImmediateComponentHolder.configurationDeleted.2" );
-                try
+                // special casing if the single component is deconfigured
+                if ( m_singleComponent == icm )
                 {
-                    // special casing if the single component is deconfigured
-                    if ( m_singleComponent == icm )
+
+                    // if the single component is the last remaining, deconfi
+                    if ( m_components.isEmpty() )
                     {
 
-                        // if the single component is the last remaining, deconfi
-                        if ( m_components.isEmpty() )
-                        {
-
-                            // if the single component is the last remaining
-                            // deconfigure it
-                            icm.reconfigure( null );
-                            dispose = false;
-
-                        }
-                        else
-                        {
-
-                            // replace the single component field with another
-                            // entry from the map
-                            m_singleComponent = ( ImmediateComponentManager ) m_components.values().iterator().next();
+                        // if the single component is the last remaining
+                        // deconfigure it
+                        icm.reconfigure( null );
+                        dispose = false;
 
-                        }
                     }
-
-                    // icm may be null if the last configuration deleted was the
-                    // single component's configuration. Otherwise the component
-                    // is not the "last" and has to be disposed off
-                    if ( dispose )
+                    else
                     {
-                        icm.disposeInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
+
+                        // replace the single component field with another
+                        // entry from the map
+                        m_singleComponent = ( ImmediateComponentManager ) m_components.values().iterator().next();
+
                     }
                 }
-                finally
+
+                // icm may be null if the last configuration deleted was the
+                // single component's configuration. Otherwise the component
+                // is not the "last" and has to be disposed off
+                if ( dispose )
                 {
-                    if ( release )
-                    {
-                        icm.releaseReadLock( "ImmediateComponentHolder.configurationDeleted.2" );
-                    }
+                    icm.disposeInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
                 }
             }
         }
@@ -285,20 +262,9 @@ public class ImmediateComponentHolder im
         if ( pid.equals( getComponentMetadata().getConfigurationPid() ) )
         {
             log( LogService.LOG_DEBUG, "ImmediateComponentHolder reconfiguring single component for pid {0} ",
-                    new Object[] {pid}, null);
-            final boolean release = m_singleComponent.obtainReadLock( "ImmediateComponentHolder.configurationUpdated.1" );
-            try
-            {
-                // singleton configuration has pid equal to component name
-                m_singleComponent.reconfigure( props );
-            }
-            finally
-            {
-                if ( release )
-                {
-                    m_singleComponent.releaseReadLock( "ImmediateComponentHolder.configurationUpdated.1" );
-                }
-            }
+                    new Object[] {pid}, null );
+            // singleton configuration has pid equal to component name
+            m_singleComponent.reconfigure( props );
         }
         else
         {
@@ -307,20 +273,9 @@ public class ImmediateComponentHolder im
             if ( icm != null )
             {
                 log( LogService.LOG_DEBUG, "ImmediateComponentHolder reconfiguring existing component for pid {0} ",
-                        new Object[] {pid}, null);
-                final boolean release = icm.obtainReadLock( "ImmediateComponentHolder.configurationUpdated.2" );
-                try
-                {
-                    // factory configuration updated for existing component instance
-                    icm.reconfigure( props );
-                }
-                finally
-                {
-                    if ( release )
-                    {
-                        icm.releaseReadLock( "ImmediateComponentHolder.configurationUpdated.2" );
-                    }
-                }
+                        new Object[] {pid}, null );
+                // factory configuration updated for existing component instance
+                icm.reconfigure( props );
             }
             else
             {
@@ -330,30 +285,19 @@ public class ImmediateComponentHolder im
                 {
                     // configure the single instance if this is not configured
                     log( LogService.LOG_DEBUG, "ImmediateComponentHolder configuring the unconfigured single component for pid {0} ",
-                            new Object[] {pid}, null);
+                            new Object[] {pid}, null );
                     newIcm = m_singleComponent;
                 }
                 else
                 {
                     // otherwise create a new instance to provide the config to
                     log( LogService.LOG_DEBUG, "ImmediateComponentHolder configuring a new component for pid {0} ",
-                            new Object[] {pid}, null);
+                            new Object[] {pid}, null );
                     newIcm = createComponentManager();
                 }
 
                 // configure the component
-                final boolean release = newIcm.obtainReadLock( "ImmediateComponentHolder.configurationUpdated.3" );
-                try
-                {
-                    newIcm.reconfigure( props );
-                }
-                finally
-                {
-                    if ( release )
-                    {
-                        newIcm.releaseReadLock( "ImmediateComponentHolder.configurationUpdated.3" );
-                    }
-                }
+                newIcm.reconfigure( props );
 
                 // enable the component if it is initially enabled
                 if ( m_enabled && getComponentMetadata().isEnabled() )

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=1402238&r1=1402237&r2=1402238&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 Oct 25 18:00:20 2012
@@ -18,9 +18,6 @@
  */
 package org.apache.felix.scr.impl.manager;
 
-import java.lang.management.ManagementFactory;
-import java.lang.management.ThreadInfo;
-import java.lang.management.ThreadMXBean;
 import java.security.Permission;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -32,9 +29,11 @@ import java.util.Hashtable;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
-import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.felix.scr.Component;
 import org.apache.felix.scr.Reference;
@@ -110,9 +109,19 @@ public abstract class AbstractComponentM
 
     private long m_timeout = 5000;
 
-    private Thread lockingThread;
-    private Throwable lockingStackTrace;
-    private ArrayList lockingActivity = new ArrayList( );
+//    private Thread lockingThread;
+//    private Throwable lockingStackTrace;
+//    private ArrayList lockingActivity = new ArrayList( );
+
+    protected volatile boolean enabled;
+    protected volatile CountDownLatch enabledLatch;
+    private final Object enabledLatchLock = new Object();
+    /**
+     * synchronizing while creating the service registration is safe as long as the bundle is not stopped
+     * during some service registrations.  So, avoid synchronizing during unregister service if the component is being
+     * disposed.
+     */
+    private volatile boolean disposed;
 
     /**
      * The constructor receives both the activator and the metadata
@@ -172,89 +181,21 @@ public abstract class AbstractComponentM
         }
     }
 
-    //ImmediateComponentHolder should be in this manager package and this should be default access.
-    public final boolean obtainReadLock( String source )
-    {
-        if ( isLogEnabled( LogService.LOG_DEBUG ) )
-        {
-            lockingActivity.add( "obtainReadLock from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
-        }
-        if (m_stateLock.getReadHoldCount() >0)
-        {
-            return false;
-        }
-        try
-        {
-            if (!m_stateLock.tryReadLock( m_timeout ) )
-            {
-                try
-                {
-                    dumpThreads();
-                }
-                catch ( Throwable e )
-                {
-                     //we are on a pre 1.6 vm, no worries
-                }
-                throw ( IllegalStateException ) new IllegalStateException( "Could not obtain lock in " + m_timeout + " milliseconds for component " + m_componentMetadata.getName() ).initCause( lockingStackTrace );
-            }
-        }
-        catch ( InterruptedException e )
-        {
-            //TODO this is so wrong
-            throw new IllegalStateException( "Could not obtain lock (Reason: " + e + ")" );
-        }
-        return true;
-    }
-
-
-    public final void releaseReadLock( String source )
-    {
-        if ( isLogEnabled( LogService.LOG_DEBUG ) )
-        {
-            lockingActivity.add( "releaseReadLock from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
-        }
-        try
-        {
-            m_stateLock.unlockReadLock();
-        }
-        catch ( IllegalMonitorStateException e )
-        {
-            logLockingInfo();
-            throw e;
-        }
-        catch ( IllegalStateException e ) //for EDU lock
-        {
-            logLockingInfo();
-            throw e;
-        }
-    }
-
-    private void logLockingInfo()
-    {
-        StringBuffer b = new StringBuffer( "Locking activity before IllegalMonitorStateException: \n" );
-        for (Iterator i = lockingActivity.iterator(); i.hasNext();)
-        {
-            b.append( "  " ).append( i.next() ).append( "\n" );
-        }
-        log( LogService.LOG_ERROR, b.toString(), null );
-        dumpThreads();
-    }
-
     final void obtainWriteLock( String source )
     {
-        if ( isLogEnabled( LogService.LOG_DEBUG ) )
-        {
-            lockingActivity.add( "obtainWriteLock from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
-        }
+//        if ( isLogEnabled( LogService.LOG_DEBUG ) )
+//        {
+//            lockingActivity.add( "obtainWriteLock from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
+//        }
         try
         {
-            if (!m_stateLock.tryWriteLock( m_timeout ) )
+            if (!m_stateLock.tryLock( m_timeout ) )
             {
-                lockingActivity.add( "obtainWriteLock failure from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis() + " Could not obtain write lock.");
+//                lockingActivity.add( "obtainWriteLock failure from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis() + " Could not obtain write lock.");
                 throw new IllegalStateException( "Could not obtain lock" );
             }
-            lockingThread = Thread.currentThread();
-            lockingStackTrace = new Exception("Write lock stack trace for thread: " + lockingThread);
+//            lockingThread = Thread.currentThread();
+//            lockingStackTrace = new Exception("Write lock stack trace for thread: " + lockingThread);
         }
         catch ( InterruptedException e )
         {
@@ -263,47 +204,20 @@ public abstract class AbstractComponentM
         }
     }
 
-    final void deescalateLock( String source )
+    final void releaseWriteLock( String source )
     {
-        if ( isLogEnabled( LogService.LOG_DEBUG ) )
-        {
-            lockingActivity.add( "deescalateLock from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
-        }
-        m_stateLock.deescalate();
-        lockingThread = null;
-        lockingStackTrace = null;
-    }
-
-    final void checkLocked()
-    {
-        if ( m_stateLock.getReadHoldCount() == 0 && m_stateLock.getWriteHoldCount() == 0 )
-        {
-            throw new IllegalStateException( "State lock should be held by current thread" );
-        }
+//        if ( isLogEnabled( LogService.LOG_DEBUG ) )
+//        {
+//            lockingActivity.add( "deescalateLock from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount() + " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread() + " time: " + System.currentTimeMillis());
+//        }
+        m_stateLock.unlock();
+//        lockingThread = null;
+//        lockingStackTrace = null;
     }
 
     final boolean isWriteLocked()
     {
-        return m_stateLock.getWriteHoldCount() > 0;
-    }
-
-    private void dumpThreads()
-    {
-        ThreadMXBean threadMXBean = ManagementFactory.getThreadMXBean();
-        StringBuffer b = new StringBuffer( "Thread dump\n" );
-        ThreadInfo[] infos = threadMXBean.dumpAllThreads( threadMXBean.isObjectMonitorUsageSupported(), threadMXBean.isSynchronizerUsageSupported() );
-        for ( int i = 0; i < infos.length; i++ )
-        {
-            ThreadInfo ti = infos[i];
-            b.append( "\n\nThreadId: " ).append( ti.getThreadId() ).append( " : name: " ).append( ti.getThreadName() ).append( " State: " ).append( ti.getThreadState() );
-            b.append( "\n  LockInfo: " ).append( ti.getLockInfo() ).append( " LockOwnerId: " ).append( ti.getLockOwnerId() ).append( " LockOwnerName: ").append( ti.getLockOwnerName() );
-            StackTraceElement[] stackTrace = ti.getStackTrace();
-            for (int j = 0; j < stackTrace.length; j++ )
-            {
-                b.append( "\n  " ).append( stackTrace[j] );
-            }
-        }
-        log( LogService.LOG_ERROR, b.toString(), null );
+        return m_stateLock.getHoldCount() > 0;
     }
 
 //---------- Component ID management
@@ -333,7 +247,7 @@ public abstract class AbstractComponentM
 
 
     //---------- Asynchronous frontend to state change methods ----------------
-
+    private static final AtomicLong taskCounter = new AtomicLong( );
     /**
      * Enables this component and - if satisfied - also activates it. If
      * enabling the component fails for any reason, the component ends up
@@ -352,46 +266,61 @@ public abstract class AbstractComponentM
 
     public final void enable( final boolean async )
     {
-        final boolean release = obtainReadLock( "AbstractComponentManager.enable.1" );
+        if (enabled)
+        {
+            return;
+        }
         try
         {
+            synchronized ( enabledLatchLock )
+            {
+                if ( enabledLatch != null )
+                {
+                    enabledLatch.await();
+                }
+                enabledLatch  = new CountDownLatch( 1 );
+            }
             enableInternal();
             if ( !async )
             {
                 activateInternal();
             }
         }
+        catch ( InterruptedException e )
+        {
+            //??
+        }
         finally
         {
-            if ( release )
+            if ( !async )
             {
-                releaseReadLock( "AbstractComponentManager.enable.1" );
+                enabledLatch.countDown();
             }
+            enabled = true;
         }
 
         if ( async )
         {
             m_activator.schedule( new Runnable()
             {
+
+                long count = taskCounter.incrementAndGet();
+
                 public void run()
                 {
-                    final boolean release = obtainReadLock( "AbstractComponentManager.enable.2" );
                     try
                     {
                         activateInternal();
                     }
                     finally
                     {
-                        if ( release )
-                        {
-                            releaseReadLock( "AbstractComponentManager.enable.2" );
-                        }
+                        enabledLatch.countDown();
                     }
                 }
 
                 public String toString()
                 {
-                    return "Async Activate: " + getComponentMetadata().getName();
+                    return "Async Activate: " + getComponentMetadata().getName() + " id: " + count;
                 }
             } );
         }
@@ -411,46 +340,61 @@ public abstract class AbstractComponentM
 
     public final void disable( final boolean async )
     {
-        final boolean release = obtainReadLock( "AbstractComponentManager.disable.1" );
+        if (!enabled)
+        {
+            return;
+        }
         try
         {
+            synchronized ( enabledLatchLock )
+            {
+                if (enabledLatch != null)
+                {
+                    enabledLatch.await();
+                }
+                enabledLatch = new CountDownLatch( 1 );
+            }
             if ( !async )
             {
-                deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED );
+                deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true );
             }
             disableInternal();
         }
+        catch ( InterruptedException e )
+        {
+            //??
+        }
         finally
         {
-            if ( release )
+            if (!async)
             {
-                releaseReadLock( "AbstractComponentManager.disable.1" );
+                enabledLatch.countDown();
             }
+            enabled = false;
         }
 
         if ( async )
         {
             m_activator.schedule( new Runnable()
             {
+
+                long count = taskCounter.incrementAndGet();
+
                 public void run()
                 {
-                    final boolean release = obtainReadLock( "AbstractComponentManager.disable.2" );
                     try
                     {
-                        deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED );
+                        deactivateInternal( ComponentConstants.DEACTIVATION_REASON_DISABLED, true );
                     }
                     finally
                     {
-                        if ( release )
-                        {
-                            releaseReadLock( "AbstractComponentManager.disable.2" );
-                        }
+                        enabledLatch.countDown();
                     }
                 }
 
                 public String toString()
                 {
-                    return "Async Deactivate: " + getComponentMetadata().getName();
+                    return "Async Deactivate: " + getComponentMetadata().getName() + " id: " + count;
                 }
 
             } );
@@ -481,18 +425,8 @@ public abstract class AbstractComponentM
      */
     public void dispose( int reason )
     {
-        final boolean release = obtainReadLock( "AbstractComponentManager.dispose.1" );
-        try
-        {
-            disposeInternal( reason );
-        }
-        finally
-        {
-            if ( release )
-            {
-                releaseReadLock( "AbstractComponentManager.dispose.1" );
-            }
-        }
+        disposed = true;
+        disposeInternal( reason );
     }
 
     //---------- Component interface ------------------------------------------
@@ -641,9 +575,9 @@ public abstract class AbstractComponentM
         return m_state.activate( this );
     }
 
-    final void deactivateInternal( int reason )
+    final void deactivateInternal( int reason, boolean disable )
     {
-        m_state.deactivate( this, reason );
+        m_state.deactivate( this, reason, disable );
     }
 
     final void disableInternal()
@@ -738,8 +672,7 @@ public abstract class AbstractComponentM
 
     protected void registerService( String[] provides )
     {
-        releaseReadLock( "register.service.1" );
-        try
+        synchronized ( m_serviceRegistration )
         {
             ServiceRegistration existing = ( ServiceRegistration ) m_serviceRegistration.get();
             if ( existing == null )
@@ -750,23 +683,20 @@ public abstract class AbstractComponentM
                 final Dictionary serviceProperties = getServiceProperties();
 
                 ServiceRegistration newRegistration = getActivator().getBundleContext().registerService(
-                    provides,
-                    getService(), serviceProperties );
-                boolean weWon = m_serviceRegistration.compareAndSet( existing, newRegistration );
-                if (weWon)
+                        provides,
+                        getService(), serviceProperties );
+                boolean weWon = !disposed && m_serviceRegistration.compareAndSet( existing, newRegistration );
+                if ( weWon )
                 {
                     return;
                 }
                 newRegistration.unregister();
             }
-            else {
+            else
+            {
                 log( LogService.LOG_DEBUG, "Existing service registration, not registering", null );
             }
         }
-        finally
-        {
-            obtainReadLock( "register.service.1" );
-        }
 
     }
 
@@ -785,20 +715,26 @@ public abstract class AbstractComponentM
 
     final void unregisterComponentService()
     {
-        ServiceRegistration sr = ( ServiceRegistration ) m_serviceRegistration.get();
-
-        if ( sr != null && m_serviceRegistration.compareAndSet( sr, null ) )
+        if ( !disposed || m_serviceRegistration.get() != null )
         {
-            log( LogService.LOG_DEBUG, "Unregistering services", null );
-            sr.unregister();
-        }
-        else if (sr == null)
-        {
-            log( LogService.LOG_DEBUG, "Service already unregistered", null);
-        }
-        else
-        {
-            log( LogService.LOG_DEBUG, "Service unregistered concurrently by another thread", null);
+            synchronized ( m_serviceRegistration )
+            {
+                ServiceRegistration sr = ( ServiceRegistration ) m_serviceRegistration.get();
+
+                if ( sr != null && m_serviceRegistration.compareAndSet( sr, null ) )
+                {
+                    log( LogService.LOG_DEBUG, "Unregistering services", null );
+                    sr.unregister();
+                }
+                else if (sr == null)
+                {
+                    log( LogService.LOG_DEBUG, "Service already unregistered", null);
+                }
+                else
+                {
+                    log( LogService.LOG_DEBUG, "Service unregistered concurrently by another thread", null);
+                }
+            }
         }
     }
 
@@ -1161,7 +1097,7 @@ public abstract class AbstractComponentM
         while ( it.hasNext() )
         {
             DependencyManager dm = (DependencyManager) it.next();
-            dm.disable();
+            dm.unregisterServiceListener();
         }
     }
 
@@ -1359,9 +1295,9 @@ public abstract class AbstractComponentM
         }
 
 
-        void deactivate( AbstractComponentManager acm, int reason )
+        void deactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
-            log( acm, "deactivate (reason: " + reason + ")" );
+            log( acm, "deactivate (reason: " + reason + ") (dsable: " + disable + ")" );
         }
 
 
@@ -1383,22 +1319,25 @@ public abstract class AbstractComponentM
                 { m_name, event, acm.m_serviceRegistration.get() }, null );
         }
 
-        void doDeactivate( AbstractComponentManager acm, int reason )
+        void doDeactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
             try
             {
-                acm.releaseReadLock( "AbstractComponentManager.State.doDeactivate.1" );
                 acm.unregisterComponentService();
                 acm.obtainWriteLock( "AbstractComponentManager.State.doDeactivate.1" );
                 try
                 {
                     acm.deleteComponent( reason );
                     acm.deactivateDependencyManagers();
+                    if ( disable )
+                    {
+                        acm.disableDependencyManagers();
+                    }
                     acm.unsetDependencyMap();
                 }
                 finally
                 {
-                    acm.deescalateLock( "AbstractComponentManager.State.doDeactivate.1" );
+                    acm.releaseWriteLock( "AbstractComponentManager.State.doDeactivate.1" );
                 }
             }
             catch ( Throwable t )
@@ -1410,7 +1349,7 @@ public abstract class AbstractComponentM
         void doDisable( AbstractComponentManager acm )
         {
             // dispose and recreate dependency managers
-            acm.disableDependencyManagers();
+//            acm.disableDependencyManagers();
 
             // reset the component id now (a disabled component has none)
             acm.unregisterComponentId();
@@ -1465,9 +1404,9 @@ public abstract class AbstractComponentM
             }
         }
 
-        void deactivate( AbstractComponentManager acm, int reason )
+        void deactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
-            doDeactivate( acm, reason );
+            doDeactivate( acm, reason, disable );
         }
 
         Object getService( ImmediateComponentManager dcm )
@@ -1521,7 +1460,7 @@ public abstract class AbstractComponentM
                 return true;
             }
 
-            acm.log( LogService.LOG_DEBUG, "Activating component", null );
+            acm.log( LogService.LOG_DEBUG, "Activating component from state ", new Object[] {this},  null );
 
             // Before creating the implementation object, we are going to
             // test if we have configuration if such is required
@@ -1572,14 +1511,12 @@ public abstract class AbstractComponentM
             // 4. Call the activate method, if present
             if ( ( acm.isImmediate() || acm.getComponentMetadata().isFactory() ) )
             {
-                acm.releaseReadLock( "AbstractComponentManager.Unsatisfied.activate.1" );
                 //don't collect dependencies for a factory component.
                 try
                 {
                     if ( !acm.collectDependencies() )
                     {
                         acm.log( LogService.LOG_DEBUG, "Not all dependencies collected, cannot create object (1)", null );
-                        acm.obtainReadLock( "AbstractComponentManager.Unsatisfied.activate.1" );
                         return false;
                     }
                     else
@@ -1592,13 +1529,11 @@ public abstract class AbstractComponentM
                 catch ( IllegalStateException e )
                 {
                     acm.log( LogService.LOG_DEBUG, "Not all dependencies collected, cannot create object (2)", null );
-                    acm.obtainReadLock( "AbstractComponentManager.Unsatisfied.activate.1" );
                     return false;
                 }
                 catch ( Throwable t )
                 {
                     acm.log( LogService.LOG_ERROR, "Unexpected throwable from attempt to collect dependencies", t );
-                    acm.obtainReadLock( "AbstractComponentManager.Unsatisfied.activate.1" );
                     return false;
                 }
                 acm.obtainWriteLock( "AbstractComponentManager.Unsatisfied.activate.1" );
@@ -1614,7 +1549,7 @@ public abstract class AbstractComponentM
                 }
                 finally
                 {
-                    acm.deescalateLock( "AbstractComponentManager.Unsatisfied.activate.1" );
+                    acm.releaseWriteLock( "AbstractComponentManager.Unsatisfied.activate.1" );
                 }
 
             }
@@ -1622,13 +1557,13 @@ public abstract class AbstractComponentM
 
         }
 
-        void deactivate( AbstractComponentManager acm, int reason )
+        void deactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
             acm.log( LogService.LOG_DEBUG, "Deactivating component", null );
 
             // catch any problems from deleting the component to prevent the
             // component to remain in the deactivating state !
-            doDeactivate(acm, reason);
+            doDeactivate(acm, reason, disable );
 
             acm.log( LogService.LOG_DEBUG, "Component deactivated", null );
         }
@@ -1646,6 +1581,7 @@ public abstract class AbstractComponentM
 
         void dispose( AbstractComponentManager acm, int reason )
         {
+            acm.disableDependencyManagers();
             doDisable( acm );
             acm.clear();   //content of Disabled.dispose
             acm.changeState( Disposed.getInstance() );
@@ -1679,13 +1615,13 @@ public abstract class AbstractComponentM
         }
 
 
-        void deactivate( AbstractComponentManager acm, int reason )
+        void deactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
             acm.log( LogService.LOG_DEBUG, "Deactivating component", null );
 
             // catch any problems from deleting the component to prevent the
             // component to remain in the deactivating state !
-            doDeactivate(acm, reason);
+            doDeactivate(acm, reason, disable );
 
             if ( acm.state().isSatisfied() )
             {
@@ -1698,11 +1634,12 @@ public abstract class AbstractComponentM
         {
             doDisable( acm );
             acm.changeState( Disabled.getInstance() );
+            acm.log( LogService.LOG_DEBUG, "Component disabled", null );
         }
 
         void dispose( AbstractComponentManager acm, int reason )
         {
-            doDeactivate( acm, reason );
+            doDeactivate( acm, reason, true );
             doDisable(acm);
             acm.clear();   //content of Disabled.dispose
             acm.changeState( Disposed.getInstance() );
@@ -1747,7 +1684,10 @@ public abstract class AbstractComponentM
         void ungetService( ImmediateComponentManager dcm )
         {
             dcm.deleteComponent( ComponentConstants.DEACTIVATION_REASON_UNSPECIFIED );
-            dcm.changeState( Registered.getInstance() );
+            if ( dcm.enabled )
+            {
+                dcm.changeState( Registered.getInstance() );
+            }
         }
     }
 
@@ -1835,7 +1775,7 @@ public abstract class AbstractComponentM
      * instances of component factory components created with the
      * <code>ComponentFactory.newInstance</code> method. This state acts the
      * same as the {@link Active} state except that the
-     * {@link #deactivate(AbstractComponentManager, int)} switches to the
+     * {@link org.apache.felix.scr.impl.manager.AbstractComponentManager.State#deactivate(AbstractComponentManager, int, boolean)} switches to the
      * real {@link Active} state before actually disposing off the component
      * because component factory instances are never reactivated after
      * deactivated due to not being satisified any longer. See section 112.5.5,
@@ -1868,7 +1808,7 @@ public abstract class AbstractComponentM
             dcm.changeState( Registered.getInstance() );
         }
 
-        void deactivate( AbstractComponentManager acm, int reason )
+        void deactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
             acm.disposeInternal( reason );
         }
@@ -1898,7 +1838,7 @@ public abstract class AbstractComponentM
             throw new IllegalStateException( "activate: " + this );
         }
 
-        void deactivate( AbstractComponentManager acm, int reason )
+        void deactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
             throw new IllegalStateException( "deactivate: " + this );
         }
@@ -1921,97 +1861,50 @@ public abstract class AbstractComponentM
 
     private static interface LockWrapper
     {
-        boolean tryReadLock( long milliseconds ) throws InterruptedException;
-        long getReadHoldCount();
-        void unlockReadLock();
-
-        boolean tryWriteLock( long milliseconds ) throws InterruptedException;
-        long getWriteHoldCount();
-        void unlockWriteLock();
-        void deescalate();
+        boolean tryLock( long milliseconds ) throws InterruptedException;
+        long getHoldCount();
+        void unlock();
 
 
     }
 
     private static class JLock implements LockWrapper
     {
-        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 tryWriteLock( long milliseconds ) throws InterruptedException
-        {
-            return lock.writeLock().tryLock( milliseconds, TimeUnit.MILLISECONDS );
-        }
+        private final ReentrantLock lock = new ReentrantLock( true );
 
-        public long getWriteHoldCount()
+        public boolean tryLock( long milliseconds ) throws InterruptedException
         {
-            return lock.getWriteHoldCount();
+            return lock.tryLock( milliseconds, TimeUnit.MILLISECONDS );
         }
 
-        public void unlockWriteLock()
+        public long getHoldCount()
         {
-            lock.writeLock().unlock();
+            return lock.getHoldCount();
         }
 
-        public void deescalate()
+        public void unlock()
         {
-            lock.readLock().lock();
-            lock.writeLock().unlock();
+            lock.unlock();
         }
     }
 
     private static class EDULock  implements LockWrapper
     {
-        private final edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantReadWriteLock lock = new edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantReadWriteLock( );
-
-        public boolean tryReadLock( long milliseconds ) throws InterruptedException
-        {
-             return lock.readLock().tryLock( milliseconds, edu.emory.mathcs.backport.java.util.concurrent.TimeUnit.MILLISECONDS );
-        }
-
-        public long getReadHoldCount()
-        {
-            return lock.getReadHoldCount();
-        }
-
-        public void unlockReadLock()
-        {
-            lock.readLock().unlock();
-        }
-
-        public boolean tryWriteLock( long milliseconds ) throws InterruptedException
-        {
-            return lock.writeLock().tryLock( milliseconds, edu.emory.mathcs.backport.java.util.concurrent.TimeUnit.MILLISECONDS );
-        }
+        private final edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock lock = new edu.emory.mathcs.backport.java.util.concurrent.locks.ReentrantLock( );
 
-        public long getWriteHoldCount()
+        public boolean tryLock( long milliseconds ) throws InterruptedException
         {
-            return lock.getWriteHoldCount();
+            return lock.tryLock( milliseconds, edu.emory.mathcs.backport.java.util.concurrent.TimeUnit.MILLISECONDS );
         }
 
-        public void unlockWriteLock()
+        public long getHoldCount()
         {
-            lock.writeLock().unlock();
+            return lock.getHoldCount();
         }
 
-        public void deescalate()
+        public void unlock()
         {
-            lock.readLock().lock();
-            lock.writeLock().unlock();
+            lock.unlock();
         }
     }
 

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=1402238&r1=1402237&r2=1402238&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 Oct 25 18:00:20 2012
@@ -102,34 +102,23 @@ public class ComponentFactoryImpl extend
     {
         final ImmediateComponentManager cm = createComponentManager();
         log( LogService.LOG_DEBUG, "Creating new instance from component factory {0} with configuration {1}",
-                new Object[] { getComponentMetadata().getName(), dictionary }, null );
+                new Object[] {getComponentMetadata().getName(), dictionary}, null );
 
         ComponentInstance instance;
-        final boolean release = cm.obtainReadLock( "ComponentFactoryImpl.newInstance.1" );
-        try
-        {
-            cm.setFactoryProperties( dictionary );
-            // enable
-            cm.enableInternal();
-            //configure the properties
-            cm.reconfigure( m_configuration );
-            //activate immediately
-            cm.activateInternal();
-
-            instance = cm.getComponentInstance();
-            if ( instance == null )
-            {
-                // activation failed, clean up component manager
-                cm.disposeInternal( ComponentConstants.DEACTIVATION_REASON_DISPOSED );
-                throw new ComponentException( "Failed activating component" );
-            }
-        }
-        finally
-        {
-            if ( release )
-            {
-                cm.releaseReadLock( "ComponentFactoryImpl.newInstance.1" );
-            }
+        cm.setFactoryProperties( dictionary );
+        // enable
+        cm.enableInternal();
+        //configure the properties
+        cm.reconfigure( m_configuration );
+        //activate immediately
+        cm.activateInternal();
+
+        instance = cm.getComponentInstance();
+        if ( instance == null )
+        {
+            // activation failed, clean up component manager
+            cm.disposeInternal( ComponentConstants.DEACTIVATION_REASON_DISPOSED );
+            throw new ComponentException( "Failed activating component" );
         }
 
         synchronized ( m_componentInstances )
@@ -323,35 +312,24 @@ public class ComponentFactoryImpl extend
         {
             log( LogService.LOG_DEBUG, "Handling configuration removal", null );
 
-            boolean release = obtainReadLock( "ComponentFactoryImpl.configurationDeleted" );
-            try
+            // nothing to do if there is no configuration currently known.
+            if ( !m_isConfigured )
             {
-                // nothing to do if there is no configuration currently known.
-                if (! m_isConfigured)
-                {
-                    log( LogService.LOG_DEBUG, "ignoring configuration removal: not currently configured", null );
-                    return;
-                }
-                
-                // So far, we were configured: clear the current configuration.
-                m_isConfigured = false;
-                m_configuration = new Hashtable();
+                log( LogService.LOG_DEBUG, "ignoring configuration removal: not currently configured", null );
+                return;
+            }
 
-                log( LogService.LOG_DEBUG, "Current component factory state={0}", new Object[] { getState() }, null );
+            // So far, we were configured: clear the current configuration.
+            m_isConfigured = false;
+            m_configuration = new Hashtable();
 
-                // And deactivate if we are not currently disposed and if configuration is required
-                if ( ( getState() & STATE_DISPOSED ) == 0 && getComponentMetadata().isConfigurationRequired() )
-                {
-                    log( LogService.LOG_DEBUG, "Deactivating component factory (required configuration has gone)", null );
-                    deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
-                }
-            }
-            finally
+            log( LogService.LOG_DEBUG, "Current component factory state={0}", new Object[] {getState()}, null );
+
+            // And deactivate if we are not currently disposed and if configuration is required
+            if ( ( getState() & STATE_DISPOSED ) == 0 && getComponentMetadata().isConfigurationRequired() )
             {
-                if ( release )
-                {
-                    releaseReadLock( "ComponentFactoryImpl.configurationDeleted" );
-                }
+                log( LogService.LOG_DEBUG, "Deactivating component factory (required configuration has gone)", null );
+                deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true );
             }
         }
         else
@@ -373,53 +351,42 @@ public class ComponentFactoryImpl extend
             {
                 return;
             }
-            
-            boolean release = obtainReadLock( "ComponentFactoryImpl.configurationUpdated" );
-            try
+
+            // Store the config admin configuration
+            m_configuration = configuration;
+
+            // We are now configured from config admin.
+            m_isConfigured = true;
+
+            log( LogService.LOG_INFO, "Current ComponentFactory state={0}", new Object[]
+                    {getState()}, null );
+
+            // If we are active, but if some config target filters don't match anymore
+            // any required references, then deactivate.
+            if ( getState() == STATE_FACTORY )
             {
-                // Store the config admin configuration
-                m_configuration = configuration;
-                
-                // We are now configured from config admin.
-                m_isConfigured = true;
-                
-                log( LogService.LOG_INFO, "Current ComponentFactory state={0}", new Object[]
-                    { getState() }, null );
-                                    
-                // If we are active, but if some config target filters don't match anymore 
-                // any required references, then deactivate.                
-                if ( getState() == STATE_FACTORY )
-                {
-                    log( LogService.LOG_INFO, "Verifying if Active Component Factory is still satisfied", null );
+                log( LogService.LOG_INFO, "Verifying if Active Component Factory is still satisfied", null );
 
-                    // First update target filters.
-                    super.updateTargets( getProperties() );
+                // First update target filters.
+                super.updateTargets( getProperties() );
 
-                    // Next, verify dependencies
-                    if ( !verifyDependencyManagers( m_configuration ) )
-                    {
-                        log( LogService.LOG_DEBUG,
+                // Next, verify dependencies
+                if ( !verifyDependencyManagers( m_configuration ) )
+                {
+                    log( LogService.LOG_DEBUG,
                             "Component Factory target filters not satisfied anymore: deactivating", null );
-                        deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
-                        return;
-                    }
-                }
-                    
-                // Unsatisfied component and required configuration may change targets
-                // to satisfy references.
-                if ( getState() == STATE_UNSATISFIED && getComponentMetadata().isConfigurationRequired() )
-                {                   
-                    // try to activate our component factory, if all dependnecies are satisfied
-                    log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
-                    activateInternal();
+                    deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false );
+                    return;
                 }
             }
-            finally
-            {
-                if (release) 
-                {
-                    releaseReadLock( "ComponentFactoryImpl.configurationUpdated" );
-                }
+
+            // Unsatisfied component and required configuration may change targets
+            // to satisfy references.
+            if ( getState() == STATE_UNSATISFIED && getComponentMetadata().isConfigurationRequired() )
+            {
+                // try to activate our component factory, if all dependnecies are satisfied
+                log( LogService.LOG_DEBUG, "Attempting to activate unsatisfied component", null );
+                activateInternal();
             }
         }
         else

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=1402238&r1=1402237&r2=1402238&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 Oct 25 18:00:20 2012
@@ -140,65 +140,42 @@ public class ConfigurationComponentFacto
         }
         else   //non-spec backwards compatible
         {
-            final boolean release = obtainReadLock( "ConfigurationComponentFactoryImpl.newInstance.1" );
-            try
+            ImmediateComponentManager cm;
+            Map configuredServices = m_configuredServices;
+            if ( configuredServices != null )
             {
-                ImmediateComponentManager cm;
-                Map configuredServices = m_configuredServices;
-                if ( configuredServices != null )
-                {
-                    cm = ( ImmediateComponentManager ) configuredServices.get( pid );
-                }
-                else
-                {
-                    m_configuredServices = new HashMap();
-                    configuredServices = m_configuredServices;
-                    cm = null;
-                }
-
-                if ( cm == null )
-                {
-                    // create a new instance with the current configuration
-                    cm = createConfigurationComponentManager();
+                cm = ( ImmediateComponentManager ) configuredServices.get( pid );
+            }
+            else
+            {
+                m_configuredServices = new HashMap();
+                configuredServices = m_configuredServices;
+                cm = null;
+            }
 
-                    // this should not call component reactivation because it is
-                    // not active yet
-                    cm.reconfigure( configuration );
-
-                    // enable asynchronously if components are already enabled
-                    if ( getState() == STATE_FACTORY )
-                    {
-                        cm.enable( false );
-                    }
+            if ( cm == null )
+            {
+                // create a new instance with the current configuration
+                cm = createConfigurationComponentManager();
 
-                    // keep a reference for future updates
-                    configuredServices.put( pid, cm );
+                // this should not call component reactivation because it is
+                // not active yet
+                cm.reconfigure( configuration );
 
-                }
-                else
+                // enable asynchronously if components are already enabled
+                if ( getState() == STATE_FACTORY )
                 {
-                    // update the configuration as if called as ManagedService
-                    //TODO deadlock potential, we are holding our own state lock.
-                    final boolean releaseInner = cm.obtainReadLock( "ConfigurationComponentFactoryImpl.newInstance.2" );
-                    try
-                    {
-                        cm.reconfigure( configuration );
-                    }
-                    finally
-                    {
-                        if ( releaseInner )
-                        {
-                            cm.releaseReadLock( "ConfigurationComponentFactoryImpl.newInstance.2" );
-                        }
-                    }
+                    cm.enable( false );
                 }
+
+                // keep a reference for future updates
+                configuredServices.put( pid, cm );
+
             }
-            finally
+            else
             {
-                if ( release )
-                {
-                    releaseReadLock( "ConfigurationComponentFactoryImpl.newInstance.1" );
-                }
+                // update the configuration as if called as ManagedService
+                cm.reconfigure( configuration );
             }
         }
     }

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=1402238&r1=1402237&r2=1402238&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 Oct 25 18:00:20 2012
@@ -21,6 +21,7 @@ package org.apache.felix.scr.impl.manage
 
 import java.security.Permission;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
 import java.util.Dictionary;
 import java.util.HashMap;
@@ -126,7 +127,6 @@ public class DependencyManager implement
         final ServiceReference ref = event.getServiceReference();
         final String serviceString = "Service " + m_dependencyMetadata.getInterface() + "/"
             + ref.getProperty( Constants.SERVICE_ID );
-        final boolean release = m_componentManager.obtainReadLock( "DependencyManager.serviceChanged.1" );
         Collection<ServiceReference> changes = null;
         try
         {
@@ -236,7 +236,11 @@ public class DependencyManager implement
                             }
                         }
                         changes = removed;
-                        m_size.decrementAndGet();
+                        if ( m_size.decrementAndGet() < 0 )
+                        {
+                            m_size.set( 0 );
+//                            throw new IllegalStateException( "Component: " + m_componentManager.getName() + ", Size less than zero for dependency manager " + getName() + " removed: " + removed );
+                        }
                         serviceRemoved( ref );
                     }
                     else
@@ -270,7 +274,11 @@ public class DependencyManager implement
                             }
                         }
                         changes = removed;
-                        m_size.decrementAndGet();
+                        if ( m_size.decrementAndGet() < 0 )
+                        {
+                            m_size.set( 0 );
+//                            throw new IllegalStateException( "Component: " + m_componentManager.getName() + ", Size less than zero for dependency manager " + getName() + " removed: " + removed );
+                        }
                         serviceRemoved( ref );
                     }
                     else
@@ -301,10 +309,6 @@ public class DependencyManager implement
                     changes.notify();
                 }
             }
-            if ( release )
-            {
-                m_componentManager.releaseReadLock( "DependencyManager.serviceChanged.1" );
-            }
         }
     }
 
@@ -360,8 +364,8 @@ public class DependencyManager implement
                 return;
             }
             //release our read lock and wait for activation to complete
-            m_componentManager.releaseReadLock( "DependencyManager.serviceAdded.nothandled.1" );
-            m_componentManager.obtainReadLock( "DependencyManager.serviceAdded.nothandled.2" );
+//            m_componentManager.releaseReadLock( "DependencyManager.serviceAdded.nothandled.1" );
+//            m_componentManager.obtainReadLock( "DependencyManager.serviceAdded.nothandled.2" );
             m_componentManager.log( LogService.LOG_DEBUG,
                     "Dependency Manager: Service {0} activation on other thread: after releasing lock, component instance is: {1}", new Object[]
                     {m_dependencyMetadata.getName(), m_componentManager.getInstance()}, null );
@@ -389,7 +393,7 @@ public class DependencyManager implement
                                             bound.isEmpty() ||
                                             reference.compareTo( bound.keySet().iterator().next() ) > 0 )
                                     {
-                                        m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
+                                        m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false );
                                         m_componentManager.activateInternal();
                                     }
                 }
@@ -452,7 +456,7 @@ public class DependencyManager implement
                     { m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface() }, null );
 
             // deactivate the component now
-            m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
+            m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false );
         }
 
         // check whether we are bound to that service, do nothing if not
@@ -464,7 +468,7 @@ public class DependencyManager implement
         }
 
         // otherwise check whether the component is in a state to handle the event
-        else if ( handleServiceEvent() )
+        else if ( handleServiceEvent() || (m_componentManager.getState() & (Component.STATE_DISABLED | Component.STATE_DISPOSED)) != 0 )
         {
             Map dependencyMap = m_componentManager.getDependencyMap();
             Map referenceMap = null;
@@ -481,7 +485,7 @@ public class DependencyManager implement
                     m_componentManager.log( LogService.LOG_DEBUG,
                         "Dependency Manager: Static dependency on {0}/{1} is broken", new Object[]
                             { m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface() }, null );
-                    m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
+                    m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false );
                     if ( referenceMap != null )
                     {
                         referenceMap.remove( reference );
@@ -519,7 +523,7 @@ public class DependencyManager implement
                                         "Dependency Manager: Deactivating component due to mandatory dependency on {0}/{1} not satisfied",
                                         new Object[]
                                                 {m_dependencyMetadata.getName(), m_dependencyMetadata.getInterface()}, null );
-                            m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE );
+                            m_componentManager.deactivateInternal( ComponentConstants.DEACTIVATION_REASON_REFERENCE, false );
                         }
                     }
                     else
@@ -643,17 +647,6 @@ public class DependencyManager implement
     }
 
 
-    /**
-     * Disposes off this dependency manager by removing as a service listener
-     * and ungetting all services, which are still kept in the list of our
-     * bound services. This list will not be empty if the service lookup
-     * method is used by the component to access the service.
-     */
-    void disable()
-    {
-        unregisterServiceListener();
-    }
-
     void deactivate()
     {
         // unget all services we once got
@@ -1535,12 +1528,11 @@ public class DependencyManager implement
      */
     private void setTargetFilter( String target ) throws InvalidSyntaxException
     {
-        m_componentManager.checkLocked();
         // do nothing if target filter does not change
         if ( ( m_target == null && target == null ) || ( m_target != null && m_target.equals( target ) ) )
         {
-            m_componentManager.log( LogService.LOG_DEBUG, "No change in target property for dependency {0}", new Object[]
-                    {m_dependencyMetadata.getName()}, null );
+            m_componentManager.log( LogService.LOG_DEBUG, "No change in target property for dependency {0}: currently registered: {1}", new Object[]
+                    {m_dependencyMetadata.getName(), registered}, null );
             if (registered)
             {
                 return;
@@ -1625,25 +1617,23 @@ public class DependencyManager implement
         synchronized ( enableLock )
         {
             // get the current number of registered services available
-            ServiceReference refs[] = getFrameworkServiceReferences();
-            if ( refs != null )
+            ServiceReference[] refArray = getFrameworkServiceReferences();
+            if ( refArray != null )
             {
+                List<ServiceReference> refs = Arrays.asList( refArray );
+                m_componentManager.log( LogService.LOG_DEBUG, "Component: {0} dependency: {1} refs: {2}", new Object[]
+                        {m_componentManager.getName(), getName(), refs}, null );
                 synchronized ( added )
                 {
-                    for ( ServiceReference ref : refs )
-                    {
-                        added.remove( ref );
-                    }
+                    m_componentManager.log( LogService.LOG_DEBUG, "Component: {0} dependency: {1} added: {2}", new Object[]
+                            {m_componentManager.getName(), getName(), added}, null );
+                    added.removeAll( refs );
                 }
                 synchronized ( removed )
                 {
-                    for ( ServiceReference ref : refs )
-                    {
-                        if ( !removed.contains( ref ) )
-                        {
-                            removed.remove( ref );
-                        }
-                    }
+                    m_componentManager.log( LogService.LOG_DEBUG, "Component: {0} dependency: {1} removed: {2}", new Object[]
+                            {m_componentManager.getName(), getName(), removed}, null );
+                    removed.retainAll( refs );
                 }
                 if ( active )
                 {
@@ -1656,7 +1646,13 @@ public class DependencyManager implement
                     }
                 }
             }
-            m_size.set( ( refs == null ) ? 0 : refs.length );
+            else
+            {
+                m_componentManager.log( LogService.LOG_DEBUG, "Component: {0} dependency: {1} no services, removed: {2}", new Object[]
+                        {m_componentManager.getName(), getName(), removed}, null );
+                removed.clear();//retainAll of empty set.
+            }
+            m_size.set( ( refArray == null ) ? 0 : refArray.length );
         }
 
         for ( ServiceReference ref : toAdd )
@@ -1671,12 +1667,16 @@ public class DependencyManager implement
         String filterString = "(" + Constants.OBJECTCLASS + "=" + m_dependencyMetadata.getInterface() + ")";
         m_componentManager.getActivator().getBundleContext().addServiceListener( this, filterString );
         registered = true;
+        m_componentManager.log( LogService.LOG_DEBUG, "registering service listener for dependency {0}", new Object[]
+                {m_dependencyMetadata.getName()}, null );
     }
 
-    private void unregisterServiceListener()
+    void unregisterServiceListener()
     {
         m_componentManager.getActivator().getBundleContext().removeServiceListener( this );
         registered = false;
+        m_componentManager.log( LogService.LOG_DEBUG, "unregistering service listener for dependency {0}", new Object[]
+                {m_dependencyMetadata.getName()}, null );
     }
 
 

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=1402238&r1=1402237&r2=1402238&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 Oct 25 18:00:20 2012
@@ -27,12 +27,9 @@ import java.util.Map;
 
 import org.apache.felix.scr.impl.BundleComponentActivator;
 import org.apache.felix.scr.impl.config.ComponentHolder;
-import org.apache.felix.scr.impl.helper.ActivateMethod;
 import org.apache.felix.scr.impl.helper.ActivateMethod.ActivatorParameter;
 import org.apache.felix.scr.impl.helper.ComponentMethods;
-import org.apache.felix.scr.impl.helper.DeactivateMethod;
 import org.apache.felix.scr.impl.helper.MethodResult;
-import org.apache.felix.scr.impl.helper.ModifiedMethod;
 import org.apache.felix.scr.impl.metadata.ComponentMetadata;
 import org.apache.felix.scr.impl.metadata.ReferenceMetadata;
 import org.osgi.framework.Bundle;
@@ -53,7 +50,7 @@ public class ImmediateComponentManager e
 {
 
     // The object that implements the service and that is bound to other services
-    private Object m_implementationObject;
+    private volatile Object m_implementationObject;
 
     // keep the using bundles as reference "counters" for instance deactivation
     private volatile int m_useCount;
@@ -133,7 +130,7 @@ public class ImmediateComponentManager e
             log( LogService.LOG_DEBUG, "Set implementation object for component {0}", new Object[] { getName() },  null );
 
             //notify that component was successfully created so any optional circular dependencies can be retried
-            getActivator().missingServicePresent(getServiceReference());
+            getActivator().missingServicePresent( getServiceReference() );
         }
         return true;
     }
@@ -490,7 +487,7 @@ public class ImmediateComponentManager e
         // this component must be deactivated
         if ( configuration == null && getComponentMetadata().isConfigurationRequired() )
         {
-            deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED );
+            deactivateInternal( ComponentConstants.DEACTIVATION_REASON_CONFIGURATION_DELETED, true );
         }
         else if ( configuration == null | !modify() )
         {
@@ -502,7 +499,7 @@ public class ImmediateComponentManager e
             // FELIX-2368: cycle component immediately, reconfigure() is
             //     called through ConfigurationListener API which itself is
             //     called asynchronously by the Configuration Admin Service
-            deactivateInternal( reason );
+            deactivateInternal( reason, false );
             activateInternal();
         }
     }
@@ -622,13 +619,9 @@ public class ImmediateComponentManager e
 
     public Object getService( Bundle bundle, ServiceRegistration serviceRegistration )
     {
-        boolean release = obtainReadLock( "ImmediateComponentManager.getService.1" );
-        try
-        {
             Object implementationObject = m_implementationObject;
             if ( implementationObject == null )
             {
-                releaseReadLock( "ImmediateComponentManager.getService.1" );
                 try
                 {
                     if ( !collectDependencies() )
@@ -654,7 +647,6 @@ public class ImmediateComponentManager e
                             LogService.LOG_INFO,
                             "Could not obtain all required dependencies, getService returning null",
                             null );
-                    release = false;
                     return null;
                 }
                 obtainWriteLock( "ImmediateComponentManager.getService.1" );
@@ -674,59 +666,39 @@ public class ImmediateComponentManager e
                 }
                 finally
                 {
-                    deescalateLock( "ImmediateComponentManager.getService.1" );
+                    releaseWriteLock( "ImmediateComponentManager.getService.1" );
                 }
             }
             m_useCount++;
             return implementationObject;
-        }
-        finally
-        {
-            if ( release )
-            {
-                releaseReadLock( "ImmediateComponentManager.getService.1" );
-            }
-        }
     }
 
     public void ungetService( Bundle bundle, ServiceRegistration serviceRegistration, Object o )
     {
-        final boolean release = obtainReadLock( "ImmediateComponentManager.ungetService.1" );
-        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() )
+        // 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() )
+            {
+                obtainWriteLock( "ImmediateComponentManager.ungetService.1" );
+                try
                 {
-                    releaseReadLock( "ImmediateComponentManager.ungetService.1" );
-                    obtainWriteLock( "ImmediateComponentManager.ungetService.1" );
-                    try
+                    if ( m_useCount == 0 )
                     {
-                        if ( m_useCount == 0 )
-                        {
-                            state().ungetService( this );
-                            unsetDependencyMap();
-                        }
-                    }
-                    finally
-                    {
-                        deescalateLock( "ImmediateComponentManager.ungetService.1" );
+                        state().ungetService( this );
+                        unsetDependencyMap();
                     }
                 }
-            }
-        }
-        finally
-        {
-            if ( release )
-            {
-                releaseReadLock( "ImmediateComponentManager.ungetService.1" );
+                finally
+                {
+                    releaseWriteLock( "ImmediateComponentManager.ungetService.1" );
+                }
             }
         }
     }

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=1402238&r1=1402237&r2=1402238&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 Oct 25 18:00:20 2012
@@ -112,10 +112,6 @@ public class ServiceFactoryComponentMana
 
         // When the getServiceMethod is called, the implementation object must be created
 
-        boolean release = obtainReadLock( "ServiceFactoryComponentManager.getService.1" );
-        try
-        {
-            releaseReadLock( "ServiceFactoryComponentManager.getService.1" );
             try
             {
                 if ( !collectDependencies() )
@@ -138,10 +134,8 @@ public class ServiceFactoryComponentMana
             catch ( IllegalStateException e )
             {
                 //cannot obtain service from a required reference
-                release = false;
                 return null;
             }
-            obtainReadLock( "ServiceFactoryComponentManager.getService.1" );
             // private ComponentContext and implementation instances
             BundleComponentContext serviceContext = new BundleComponentContext( this, bundle );
             Object service = createImplementationObject( serviceContext );
@@ -167,14 +161,6 @@ public class ServiceFactoryComponentMana
             }
 
             return service;
-        }
-        finally
-        {
-            if ( release )
-            {
-                releaseReadLock( "ServiceFactoryComponentManager.getService.1" );
-            }
-        }
     }
 
 
@@ -186,28 +172,17 @@ public class ServiceFactoryComponentMana
         log( LogService.LOG_DEBUG, "ServiceFactory.ungetService()", null );
 
         // When the ungetServiceMethod is called, the implementation object must be deactivated
-        final boolean release = obtainReadLock( "ServiceFactoryComponentManager.ungetService.1" );
-        try
-        {
-// private ComponentContext and implementation instances
-            final ComponentContext serviceContext;
-            serviceContext = ( ComponentContext ) serviceContexts.remove( service );
+        // private ComponentContext and implementation instances
+        final ComponentContext serviceContext;
+        serviceContext = ( ComponentContext ) serviceContexts.remove( service );
 
-            disposeImplementationObject( service, serviceContext, ComponentConstants.DEACTIVATION_REASON_DISPOSED );
+        disposeImplementationObject( service, serviceContext, ComponentConstants.DEACTIVATION_REASON_DISPOSED );
 
-            // if this was the last use of the component, go back to REGISTERED state
-            if ( serviceContexts.isEmpty() && getState() == STATE_ACTIVE )
-            {
-                changeState( Registered.getInstance() );
-                unsetDependencyMap();
-            }
-        }
-        finally
+        // if this was the last use of the component, go back to REGISTERED state
+        if ( serviceContexts.isEmpty() && getState() == STATE_ACTIVE )
         {
-            if ( release )
-            {
-                releaseReadLock( "ServiceFactoryComponentManager.ungetService.1" );
-            }
+            changeState( Registered.getInstance() );
+            unsetDependencyMap();
         }
     }