felix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From djen...@apache.org
Subject svn commit: r1381408 - in /felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager: AbstractComponentManager.java DependencyManager.java ImmediateComponentManager.java
Date Wed, 05 Sep 2012 21:49:36 GMT
Author: djencks
Date: Wed Sep  5 21:49:35 2012
New Revision: 1381408

URL: http://svn.apache.org/viewvc?rev=1381408&view=rev
Log:
FELIX-3645 Finish unlocking around all calls to register service and obtain dependent services.
 Some cleanup.

Modified:
    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/DependencyManager.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ImmediateComponentManager.java

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=1381408&r1=1381407&r2=1381408&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
Wed Sep  5 21:49:35 2012
@@ -91,7 +91,9 @@ public abstract class AbstractComponentM
     // The dependency managers that manage every dependency
     private final List m_dependencyManagers;
 
-    //<Map<DependencyManager, Map<ServiceReference, Object[]>>>
+    private boolean m_dependencyManagersInitialized;
+
+    //<Map<DependencyManager, Map<ServiceReference, RefPair>>>
     private final AtomicReferenceWrapper m_dependencies_map;
 
     // A reference to the BundleComponentActivator
@@ -216,15 +218,14 @@ public abstract class AbstractComponentM
         }
     }
 
-    final void escalateLock( String source )
+    final void obtainWriteLock( String source )
     {
-        lockingActivity.add( "escalateLock from: " +  source + " readLocks: " + m_stateLock.getReadHoldCount()
+ " writeLocks: " + m_stateLock.getWriteHoldCount() + " thread: " + Thread.currentThread()
+ " time: " + System.currentTimeMillis());
-        m_stateLock.unlockReadLock();
+        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 ) )
             {
-                lockingActivity.add( "escalateLock 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();
@@ -694,24 +695,33 @@ public abstract class AbstractComponentM
 
     protected void registerService( String[] provides )
     {
-        ServiceRegistration existing = ( ServiceRegistration ) m_serviceRegistration.get();
-        if ( existing == null )
+        releaseReadLock( "register.service.1" );
+        try
         {
-            log( LogService.LOG_DEBUG, "registering services", null );
+            ServiceRegistration existing = ( ServiceRegistration ) 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();
+                // get a copy of the component properties as service properties
+                final Dictionary serviceProperties = getServiceProperties();
 
-            ServiceRegistration newRegistration = getActivator().getBundleContext().registerService(
-                provides,
-                getService(), serviceProperties );
-            boolean weWon = m_serviceRegistration.compareAndSet( existing, newRegistration
);
-            if (weWon)
-            {
-                return;
+                ServiceRegistration newRegistration = getActivator().getBundleContext().registerService(
+                    provides,
+                    getService(), serviceProperties );
+                boolean weWon = m_serviceRegistration.compareAndSet( existing, newRegistration
);
+                if (weWon)
+                {
+                    return;
+                }
+                newRegistration.unregister();
             }
-            newRegistration.unregister();
         }
+        finally
+        {
+            obtainReadLock( "register.service.1" );
+        }
+
     }
 
     /**
@@ -738,15 +748,13 @@ public abstract class AbstractComponentM
         }
     }
 
-    protected boolean collectDependencies()
+    boolean initDependencyManagers()
     {
-        Map old = ( Map ) m_dependencies_map.get();
-        if ( old != null)
+        if ( m_dependencyManagersInitialized )
         {
-            log( LogService.LOG_DEBUG, "dependency map already present, do not collect dependencies",
null );
-            return false;
+            return true;
         }
-        Class implementationObjectClass = null;
+        Class implementationObjectClass;
         try
         {
             implementationObjectClass = getActivator().getBundleContext().getBundle().loadClass(
@@ -757,19 +765,46 @@ public abstract class AbstractComponentM
             log( LogService.LOG_ERROR, "Could not load implementation object class", e );
             return false;
         }
-        Map newDeps = new HashMap( );//<DependencyManager, Map<ServiceReference, RefPair>
         for (Iterator it = m_dependencyManagers.iterator(); it.hasNext(); )
         {
             DependencyManager dependencyManager = ( DependencyManager ) it.next();
 
             dependencyManager.initBindingMethods( implementationObjectClass );
+        }
+        m_dependencyManagersInitialized = true;
+        return true;
+    }
+
+    /**
+     * Collect and store in m_dependencies_map all the services for dependencies, outside
of any locks.
+     * Throwing IllegalStateException on failure to collect all the dependencies is needed
so getService can
+     * know to return null.
+     *
+     * @return true if this thread collected the dependencies;
+     *   false if some other thread successfully collected the dependencies;
+     * @throws IllegalStateException if some dependency is no longer available.
+     */
+    protected boolean collectDependencies() throws IllegalStateException
+    {
+        Map old = ( Map ) m_dependencies_map.get();
+        if ( old != null)
+        {
+            log( LogService.LOG_DEBUG, "dependency map already present, do not collect dependencies",
null );
+            return false;
+        }
+        initDependencyManagers();
+        Map newDeps = new HashMap( );//<DependencyManager, Map<ServiceReference, RefPair>
+        for (Iterator it = m_dependencyManagers.iterator(); it.hasNext(); )
+        {
+            DependencyManager dependencyManager = ( DependencyManager ) it.next();
+
             if (!dependencyManager.prebind( newDeps) )
             {
                 //not actually satisfied any longer
                 returnServices( newDeps );
                 log( LogService.LOG_DEBUG, "Could not get dependency for dependency manager:
{0}",
                         new Object[] {dependencyManager}, null );
-                return false;
+                throw new IllegalStateException( "Missing dependencies, not satisfied" );
             }
         }
         if ( !setDependencyMap( old, newDeps ) )
@@ -871,7 +906,7 @@ public abstract class AbstractComponentM
         m_dependencyManagers.clear();
     }
 
-    //<DepeendencyManager, Map<ServiceReference, Object[]>>
+    //<DependencyManager, Map<ServiceReference, RefPair>>
     protected Map getParameterMap()
     {
         return ( Map ) m_dependencies_map.get();
@@ -1246,22 +1281,18 @@ public abstract class AbstractComponentM
 
         ServiceReference getServiceReference( AbstractComponentManager acm )
         {
-//            return null;
             throw new IllegalStateException("getServiceReference" + this);
         }
 
 
         Object getService( ImmediateComponentManager dcm )
         {
-//            log( dcm, "getService" );
-//            return null;
             throw new IllegalStateException("getService" + this);
         }
 
 
         void ungetService( ImmediateComponentManager dcm )
         {
-//            log( dcm, "ungetService" );
             throw new IllegalStateException("ungetService" + this);
         }
 
@@ -1269,7 +1300,6 @@ public abstract class AbstractComponentM
         void enable( AbstractComponentManager acm )
         {
             log( acm, "enable" );
-//            throw new IllegalStateException("enable" + this);
         }
 
 
@@ -1277,27 +1307,23 @@ public abstract class AbstractComponentM
         {
             log( acm, "activate" );
             return false;
-//            throw new IllegalStateException("activate" + this);
         }
 
 
         void deactivate( AbstractComponentManager acm, int reason )
         {
             log( acm, "deactivate (reason: " + reason + ")" );
-//            throw new IllegalStateException("deactivate" + this);
         }
 
 
         void disable( AbstractComponentManager acm )
         {
-//            log( acm, "disable" );
             throw new IllegalStateException("disable" + this);
         }
 
 
         void dispose( AbstractComponentManager acm, int reason )
         {
-//            log( acm, "dispose (reason: " + reason + ")" );
             throw new IllegalStateException("dispose" + this);
         }
 
@@ -1312,8 +1338,9 @@ public abstract class AbstractComponentM
         {
             try
             {
+                acm.releaseReadLock( "AbstractComponentManager.State.doDeactivate.1" );
                 acm.unregisterComponentService();
-                acm.escalateLock( "AbstractComponentManager.State.doDeactivate.1" );
+                acm.obtainWriteLock( "AbstractComponentManager.State.doDeactivate.1" );
                 try
                 {
                     acm.deleteComponent( reason );
@@ -1477,13 +1504,24 @@ 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.
-                 if ( !acm.collectDependencies() )
-                 {
-                     acm.log( LogService.LOG_DEBUG, "Not all dependencies collected, cannot
create object", null );
-                     return false;
-                 }
-                acm.escalateLock( "AbstractComponentManager.Unsatisifed.activate.1" );
+                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;
+                    }
+                }
+                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;
+                }
+                acm.obtainWriteLock( "AbstractComponentManager.Unsatisfied.activate.1" );
                 try
                 {
                     acm.changeState( acm.getActiveState() );
@@ -1496,7 +1534,7 @@ public abstract class AbstractComponentM
                 }
                 finally
                 {
-                    acm.deescalateLock( "AbstractComponentManager.Unsatisifed.activate.1"
);
+                    acm.deescalateLock( "AbstractComponentManager.Unsatisfied.activate.1"
);
                 }
 
             }

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=1381408&r1=1381407&r2=1381408&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
Wed Sep  5 21:49:35 2012
@@ -59,18 +59,12 @@ public class DependencyManager implement
     private static final int STATE_MASK = //Component.STATE_UNSATISFIED |
          Component.STATE_ACTIVE | Component.STATE_REGISTERED | Component.STATE_FACTORY;
 
-    // pseudo service to mark a bound service without actual service instance
-//    private static final Object BOUND_SERVICE_SENTINEL = new Object();
-
     // the component to which this dependency belongs
     private final AbstractComponentManager m_componentManager;
 
     // Reference to the metadata
     private final ReferenceMetadata m_dependencyMetadata;
 
-    // The map of bound services indexed by their ServiceReference
-//    private final Map m_bound;
-
     // the number of matching services registered in the system
     private volatile int m_size;
 
@@ -102,8 +96,6 @@ public class DependencyManager implement
     {
         m_componentManager = componentManager;
         m_dependencyMetadata = dependency;
-//        m_bound = Collections.synchronizedMap( new HashMap() );
-
 
         // dump the reference information if DEBUG is enabled
         if ( m_componentManager.isLogEnabled( LogService.LOG_DEBUG ) )
@@ -321,8 +313,8 @@ public class DependencyManager implement
                 return;
             }
             //release our read lock and wait for activation to complete
-            m_componentManager.escalateLock( "DependencyManager.serviceAdded.nothandled.1"
);
-            m_componentManager.deescalateLock( "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
);
@@ -613,12 +605,15 @@ public class DependencyManager implement
     void deactivate()
     {
         // unget all services we once got
-        ServiceReference[] boundRefs = getBoundServiceReferences();
-        if ( boundRefs != null )
+        if ( m_componentManager.getDependencyMap() != null )
         {
-            for ( int i = 0; i < boundRefs.length; i++ )
+            ServiceReference[] boundRefs = getBoundServiceReferences();
+            if ( boundRefs != null )
             {
-                ungetService( boundRefs[i] );
+                for ( int i = 0; i < boundRefs.length; i++ )
+                {
+                    ungetService( boundRefs[i] );
+                }
             }
         }
     }
@@ -807,32 +802,17 @@ public class DependencyManager implement
      */
     private boolean isBound()
     {
-        Map bound = ( Map ) m_componentManager.getDependencyMap().get( this );
+        Map dependencyMap = m_componentManager.getDependencyMap();
+        if (dependencyMap  == null )
+        {
+            return false;
+        }
+        Map bound = ( Map ) dependencyMap.get( this );
         return !bound.isEmpty();
     }
 
 
     /**
-     * Adds the {@link #BOUND_SERVICE_SENTINEL} object as a pseudo service to
-     * the map of bound services. This method allows keeping track of services
-     * which have been bound but not retrieved from the service registry, which
-     * is the case if the bind method is called with a ServiceReference instead
-     * of the service object itself.
-     * <p>
-     * We have to keep track of all services for which we called the bind
-     * method to be able to call the unbind method in case the service is
-     * unregistered.
-     *
-     * @param serviceReference The reference to the service being marked as
-     *      bound.
-     */
-//    private void bindService( ServiceReference serviceReference )
-//    {
-//        m_bound.put( serviceReference, BOUND_SERVICE_SENTINEL );
-//    }
-
-
-    /**
      * Returns the RefPair containing the given service reference and the bound service
      * or <code>null</code> if this is instance is not currently bound to that
      * service.
@@ -890,7 +870,7 @@ public class DependencyManager implement
             return null;
         }
 
-        // keep the service for latter ungetting
+        // keep the service for later ungetting
         if ( serviceObject != null )
         {
             if (refPair != null)
@@ -989,7 +969,6 @@ public class DependencyManager implement
 
     boolean open( Object instance, Map parameters )
     {
-//        initBindingMethods( instance.getClass() );
         m_componentInstance = instance;
         return bind(parameters);
     }
@@ -1008,9 +987,6 @@ public class DependencyManager implement
         finally
         {
             m_componentInstance = null;
-//            m_bind = null;
-//            m_unbind = null;
-//            m_updated = null;
         }
     }
 
@@ -1172,16 +1148,34 @@ public class DependencyManager implement
     private boolean invokeBindMethod( ServiceReference ref )
     {
         //event driven, and we already checked this ref is not yet handled.
-        Map dependencyMap = m_componentManager.getDependencyMap();
-        if ( dependencyMap != null )
+        if ( m_componentInstance != null )
         {
-            Map deps = ( Map ) dependencyMap.get( this );
-            BundleContext bundleContext = m_componentManager.getActivator().getBundleContext();
-            AbstractComponentManager.RefPair refPair = m_bind.getServiceObject( ref, bundleContext
);
-            deps.put( ref, refPair );
-            return invokeBindMethod( refPair );
+            Map dependencyMap = m_componentManager.getDependencyMap();
+            if ( dependencyMap != null )
+            {
+                if (m_bind == null)
+                {
+                    m_componentManager.log( LogService.LOG_ERROR,
+                        "For dependency {0}, bind method not set: component state {1}",
+                        new Object[]
+                            { new Integer(m_componentManager.getState())  }, null );
+
+                }
+                Map deps = ( Map ) dependencyMap.get( this );
+                BundleContext bundleContext = m_componentManager.getActivator().getBundleContext();
+                AbstractComponentManager.RefPair refPair = m_bind.getServiceObject( ref,
bundleContext );
+                deps.put( ref, refPair );
+                return invokeBindMethod( refPair );
+            }
+            return false;
+        }
+        else
+        {
+            m_componentManager.log( LogService.LOG_DEBUG,
+                "DependencyManager : component not yet created, assuming bind method call
succeeded",
+                null );
+            return true;
         }
-        return false;
     }
 
     /**

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=1381408&r1=1381407&r2=1381408&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
Wed Sep  5 21:49:35 2012
@@ -615,20 +615,29 @@ public class ImmediateComponentManager e
 
     public Object getService( Bundle bundle, ServiceRegistration serviceRegistration )
     {
-        final boolean release = obtainReadLock( "ImmediateComponentManager.getService.1"
);
+        boolean release = obtainReadLock( "ImmediateComponentManager.getService.1" );
         try
         {
             if ( m_useCount == 0 )
             {
-                if ( !collectDependencies() )
+                releaseReadLock( "ImmediateComponentManager.getService.1" );
+                try
                 {
-                    log(
-                            LogService.LOG_INFO,
-                            "getService did not win collecting dependencies, try creating
object anyway.",
-                            null );
+                    if ( !collectDependencies() )
+                    {
+                        log(
+                                LogService.LOG_INFO,
+                                "getService did not win collecting dependencies, try creating
object anyway.",
+                                null );
 
+                    }
+                }
+                catch ( IllegalStateException e )
+                {
+                    release = false;
+                    return null;
                 }
-                escalateLock( "ImmediateComponentManager.getService.1" );
+                obtainWriteLock( "ImmediateComponentManager.getService.1" );
                 try
                 {
                     if ( m_useCount == 0 )
@@ -675,7 +684,8 @@ public class ImmediateComponentManager e
                 // be kept (FELIX-3039)
                 if ( m_useCount == 0 && !isImmediate() && !getActivator().getConfiguration().keepInstances()
)
                 {
-                    escalateLock( "ImmediateComponentManager.ungetService.1" );
+                    releaseReadLock( "ImmediateComponentManager.ungetService.1" );
+                    obtainWriteLock( "ImmediateComponentManager.ungetService.1" );
                     try
                     {
                         if ( m_useCount == 0 )



Mime
View raw message