felix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From djen...@apache.org
Subject svn commit: r1444489 - in /felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager: AbstractComponentManager.java ComponentFactoryImpl.java
Date Sun, 10 Feb 2013 07:43:16 GMT
Author: djencks
Date: Sun Feb 10 07:43:16 2013
New Revision: 1444489

URL: http://svn.apache.org/r1444489
Log:
FELIX-3891 unlock around service registration/unregistration

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/ComponentFactoryImpl.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=1444489&r1=1444488&r2=1444489&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
Sun Feb 10 07:43:16 2013
@@ -30,9 +30,11 @@ import java.util.Set;
 import java.util.TreeSet;
 import java.util.concurrent.CountDownLatch;
 import java.util.concurrent.TimeUnit;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
+import java.util.concurrent.locks.Lock;
 import java.util.concurrent.locks.ReentrantLock;
 
 import org.apache.felix.scr.Component;
@@ -96,7 +98,10 @@ public abstract class AbstractComponentM
     private BundleComponentActivator m_activator;
 
     // The ServiceRegistration
-    private final AtomicReference<ServiceRegistration<S>> m_serviceRegistration;
+    private enum RegState {unregistered, registered};
+    private RegState m_desiredServiceRegistrationFlag;
+    private RegState m_serviceRegistrationFlag;
+    private volatile ServiceRegistration<S> m_serviceRegistration;
 
     private final ReentrantLock m_stateLock;
 
@@ -142,7 +147,6 @@ public abstract class AbstractComponentM
         m_dependencyManagers = loadDependencyManagers( metadata );
 
         m_stateLock = new ReentrantLock( true );
-        m_serviceRegistration = new AtomicReference<ServiceRegistration<S>>();
 
         // dump component details
         if ( isLogEnabled( LogService.LOG_DEBUG ) )
@@ -701,87 +705,132 @@ public abstract class AbstractComponentM
     {
         return m_componentMethods;
     }
+    
+    protected String[] getProvidedServices()
+    {
+        if ( getComponentMetadata().getServiceMetadata() != null )
+        {
+            String[] provides = getComponentMetadata().getServiceMetadata().getProvides();
+            return provides;
+        }
+        return null;
+        
+    }
     /**
      * Registers the service on behalf of the component.
      *
-     * @return The <code>ServiceRegistration</code> for the registered
-     *      service or <code>null</code> if no service is registered.
      */
-    protected void registerService()
+    protected boolean registerService()
     {
-        if ( getComponentMetadata().getServiceMetadata() != null )
+        String[] services = getProvidedServices();
+        if ( services != null )
         {
-            String[] provides = getComponentMetadata().getServiceMetadata().getProvides();
-            registerService( provides );
+            return changeRegistration( RegState.registered, services);
         }
+        return true;
     }
 
-    protected void registerService( String[] provides )
+    protected boolean unregisterService()
     {
-        synchronized ( m_serviceRegistration )
+        String[] services = getProvidedServices();
+        if ( services != null )
         {
-            ServiceRegistration existing = m_serviceRegistration.get();
-            if ( existing == null )
-            {
-                log( LogService.LOG_DEBUG, "registering services", null );
-
-                // get a copy of the component properties as service properties
-                final Dictionary<String, Object> serviceProperties = getServiceProperties();
-
-                ServiceRegistration newRegistration = getActivator().getBundleContext().registerService(
-                        provides,
-                        getService(), serviceProperties );
-                boolean weWon = !disposed && m_serviceRegistration.compareAndSet(
existing, newRegistration );
-                if ( weWon )
-                {
-                    return;
-                }
-                newRegistration.unregister();
-            }
-            else
-            {
-                log( LogService.LOG_DEBUG, "Existing service registration, not registering",
null );
-            }
+            return changeRegistration( RegState.unregistered, services );
         }
-
+        return true;
     }
-
+    
+    private final Lock lock = new ReentrantLock();
+    
     /**
-     * Registers the service on behalf of the component using the
-     * {@link #registerService()} method. Also records the service
-     * registration for later {@link #unregisterComponentService()}.
-     * <p>
-     * Due to more extensive locking FELIX-3317 is no longer relevant.
-     *
+     * 
+     * @param desired
+     * @return true if this thread reached the requested state
      */
-    final void registerComponentService()
-    {
-        registerService();
-    }
-
-    final void unregisterComponentService()
+    private boolean changeRegistration( RegState desired, String[] services )
     {
-        if ( !disposed || m_serviceRegistration.get() != null )
+        lock.lock();
+        try
         {
-            synchronized ( m_serviceRegistration )
-            {
-                ServiceRegistration sr = m_serviceRegistration.get();
-
-                if ( sr != null && m_serviceRegistration.compareAndSet( sr, null
) )
+            do {
+                if ( desired.equals(m_desiredServiceRegistrationFlag))
                 {
-                    log( LogService.LOG_DEBUG, "Unregistering services", null );
-                    sr.unregister();
+                    //another thread is already trying the same state change
+                    return false;
+                }
+                m_desiredServiceRegistrationFlag = desired;
+                if ( desired.equals(m_serviceRegistrationFlag))
+                {
+                    //already in the desired state but leaving it
+                    continue;
                 }
-                else if (sr == null)
+
+                if (desired.equals( RegState.registered ))
                 {
-                    log( LogService.LOG_DEBUG, "Service already unregistered", null);
+                    log( LogService.LOG_DEBUG, "registering services", null );
+
+                    // get a copy of the component properties as service properties
+                    final Dictionary<String, Object> serviceProperties = getServiceProperties();
+
+                    ServiceRegistration<S> serviceRegistration = null;
+                    lock.unlock();
+                    try {
+                        serviceRegistration = ( ServiceRegistration<S> ) getActivator().getBundleContext().registerService(
+                                services,
+                                getService(), serviceProperties );
+                    }
+                    finally
+                    {
+                        lock.lock();
+                    }
+                    if ( desired.equals(m_desiredServiceRegistrationFlag ) && !disposed
)
+                    {
+                        m_serviceRegistration = serviceRegistration;
+                        m_serviceRegistrationFlag = desired;
+                        return true;
+                    }
+                    else 
+                    {
+                        log( LogService.LOG_DEBUG, "unregistering services after concurrent
registration", null );
+                        m_serviceRegistrationFlag = RegState.unregistered;
+                        lock.unlock();
+                        try
+                        {
+                            serviceRegistration.unregister();
+                        }
+                        finally
+                        {
+                            lock.lock();
+                        }
+                    }
+
                 }
                 else
                 {
-                    log( LogService.LOG_DEBUG, "Service unregistered concurrently by another
thread", null);
+                    ServiceRegistration<S> serviceRegistration = m_serviceRegistration;
+                    m_serviceRegistration = null;
+                    m_serviceRegistrationFlag = desired;
+                    lock.unlock();
+                    try
+                    {
+                        serviceRegistration.unregister();
+                    }
+                    finally
+                    {
+                        lock.lock();
+                    }
+                    if ( desired.equals(m_desiredServiceRegistrationFlag ) )
+                    {
+                        return true;
+                    }
                 }
-            }
+            } while (true);
         }
+        finally
+        {
+            lock.unlock();
+        }
+
     }
 
     AtomicInteger getTrackingCount()
@@ -885,7 +934,7 @@ public abstract class AbstractComponentM
 
     final ServiceRegistration<?> getServiceRegistration()
     {
-        return m_serviceRegistration.get();
+        return m_serviceRegistration;
     }
 
 
@@ -1201,7 +1250,7 @@ public abstract class AbstractComponentM
     void changeState( State newState )
     {
         log( LogService.LOG_DEBUG, "State transition : {0} -> {1} : service reg: {2}",
new Object[]
-            { m_state, newState, m_serviceRegistration.get() }, null );
+            { m_state, newState, m_serviceRegistration }, null );
         m_state = newState;
     }
 
@@ -1313,14 +1362,14 @@ public abstract class AbstractComponentM
         private void log( AbstractComponentManager acm, String event )
         {
             acm.log( LogService.LOG_DEBUG, "Current state: {0}, Event: {1}, Service registration:
{2}", new Object[]
-                { m_name, event, acm.m_serviceRegistration.get() }, null );
+                { m_name, event, acm.m_serviceRegistration }, null );
         }
 
         void doDeactivate( AbstractComponentManager acm, int reason, boolean disable )
         {
             try
             {
-                acm.unregisterComponentService();
+                acm.unregisterService();
                 acm.obtainWriteLock( "AbstractComponentManager.State.doDeactivate.1" );
                 try
                 {
@@ -1445,7 +1494,7 @@ public abstract class AbstractComponentM
                 return true;
             }
 
-            acm.log( LogService.LOG_DEBUG, "Activating component from state ", new Object[]
{this},  null );
+            acm.log( LogService.LOG_DEBUG, "Activating component from state {0}", new Object[]
{this},  null );
 
             // Before creating the implementation object, we are going to
             // test if we have configuration if such is required
@@ -1488,7 +1537,7 @@ public abstract class AbstractComponentM
             final State satisfiedState = acm.getSatisfiedState();
             acm.changeState( satisfiedState );
 
-            acm.registerComponentService();
+            acm.registerService();
 
             // 1. Load the component implementation class
             // 2. Create the component instance and component context

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=1444489&r1=1444488&r2=1444489&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
Sun Feb 10 07:43:16 2013
@@ -186,11 +186,10 @@ public class ComponentFactoryImpl<S> ext
     }
 
 
-    protected void registerService()
+    @Override
+    protected String[] getProvidedServices()
     {
-        log( LogService.LOG_DEBUG, "registering component factory", null );
-        registerService(new String[]
-            { ComponentFactory.class.getName() });
+        return new String[] { ComponentFactory.class.getName() };
     }
 
 



Mime
View raw message