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 2016/05/21 04:46:03 UTC

svn commit: r1744827 - in /felix/trunk/scr/src: main/java/org/apache/felix/scr/impl/ main/java/org/apache/felix/scr/impl/manager/ test/java/org/apache/felix/scr/integration/

Author: djencks
Date: Sat May 21 04:46:03 2016
New Revision: 1744827

URL: http://svn.apache.org/viewvc?rev=1744827&view=rev
Log:
FELIX-4417 More informative circular reference tracking

Modified:
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentActivator.java
    felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
    felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/CircularReferenceTest.java

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java?rev=1744827&r1=1744826&r2=1744827&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/BundleComponentActivator.java Sat May 21 04:46:03 2016
@@ -839,6 +839,16 @@ public class BundleComponentActivator im
             }
         }
     }
+    
+    public <T> boolean enterCreate(ServiceReference<T> serviceReference)
+    {
+        return m_componentRegistry.enterCreate(serviceReference);
+    }
+    
+    public <T> void leaveCreate(ServiceReference<T> serviceReference)
+    {
+        m_componentRegistry.leaveCreate(serviceReference);
+    }
 
     public <T> void missingServicePresent(ServiceReference<T> serviceReference)
     {

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java?rev=1744827&r1=1744826&r2=1744827&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/ComponentRegistry.java Sat May 21 04:46:03 2016
@@ -471,21 +471,76 @@ public class ComponentRegistry
         // fall back: bundle is not considered active
         return false;
     }
+    
+    private final ThreadLocal<List<ServiceReference<?>>> circularInfos = new ThreadLocal<List<ServiceReference<?>>> (); 
+    
+    
+    public <T> boolean enterCreate(final ServiceReference<T> serviceReference)
+    {
+        List<ServiceReference<?>> info = circularInfos.get();
+        if (info == null) {
+            circularInfos.set(info = new ArrayList<ServiceReference<?>>());
+        }
+        if (info.contains(serviceReference))
+        {
+            m_logger.log(LogService.LOG_ERROR,
+                "Circular reference detected trying to get service {0}: stack of references: {1}",
+                new Object[] {serviceReference, info},
+                null);
+            return true;
+        }
+        m_logger.log(LogService.LOG_DEBUG,
+            "getService  {0}: stack of references: {1}",
+            new Object[] {serviceReference, info},
+            null);
+        info.add(serviceReference);
+        return false;
+    }
+    
+    public <T> void leaveCreate(final ServiceReference<T> serviceReference)
+    {
+        List<ServiceReference<?>> info = circularInfos.get();
+        if (info != null)
+        {
+            if (!info.isEmpty() && info.iterator().next().equals(serviceReference))
+            {
+                circularInfos.remove();
+            }
+            else
+            {
+                info.remove(serviceReference);
+            } 
+        }
+        
+    }
 
+    /**
+     * Schedule late binding of now-available reference on a different thread.  The late binding cannot occur on this thread
+     * due to service registry circular reference detection. We cannot wait for the late binding before returning from the initial
+     * getService call because of synchronization in the service registry.
+     * @param serviceReference
+     * @param actor
+     */
     public synchronized <T> void missingServicePresent( final ServiceReference<T> serviceReference, ComponentActorThread actor )
     {
         final List<Entry<?, ?>> dependencyManagers = m_missingDependencies.remove( serviceReference );
         if ( dependencyManagers != null )
         {
-            actor.schedule( new Runnable()
+            
+            Runnable runnable = new Runnable()
             {
 
+                @SuppressWarnings("unchecked")
                 public void run()
                 {
                     for ( Entry<?, ?> entry : dependencyManagers )
                     {
                         ((DependencyManager<?, T>)entry.getDm()).invokeBindMethodLate( serviceReference, entry.getTrackingCount() );
                     }
+                    m_logger.log(LogService.LOG_DEBUG,
+                        "Ran {0} asynchronously",
+                        new Object[] {this},
+                        null);
                 }
 
                 @Override
@@ -494,7 +549,12 @@ public class ComponentRegistry
                     return "Late binding task of reference " + serviceReference + " for dependencyManagers " + dependencyManagers;
                 }
 
-            } );
+            } ;
+            m_logger.log(LogService.LOG_DEBUG,
+                "Scheduling runnable {0} asynchronously",
+                new Object[] {runnable},
+                null);
+            actor.schedule( runnable );
         }
     }
 
@@ -503,6 +563,10 @@ public class ComponentRegistry
         //check that the service reference is from scr
         if ( serviceReference.getProperty( ComponentConstants.COMPONENT_NAME ) == null || serviceReference.getProperty( ComponentConstants.COMPONENT_ID ) == null )
         {
+            m_logger.log(LogService.LOG_DEBUG,
+                "Missing service {0} for dependency manager {1} is not a DS service, cannot resolve circular dependency",
+                new Object[] {serviceReference, dependencyManager},
+                null);
             return;
         }
         List<Entry<?, ?>> dependencyManagers = m_missingDependencies.get( serviceReference );
@@ -512,7 +576,11 @@ public class ComponentRegistry
             m_missingDependencies.put( serviceReference, dependencyManagers );
         }
         dependencyManagers.add( new Entry<S, T>( dependencyManager, trackingCount ) );
-    }
+        m_logger.log(LogService.LOG_DEBUG,
+            "Dependency managers {0} waiting for missing service {1}",
+            new Object[] {dependencyManagers, serviceReference},
+            null);
+        }
 
     private static class Entry<S,T>
     {
@@ -534,6 +602,12 @@ public class ComponentRegistry
         {
             return trackingCount;
         }
+        
+        @Override
+        public String toString() 
+        {
+            return dm.toString() + "@" + trackingCount;
+        }
     }
     
     private final ConcurrentMap<Long, RegionConfigurationSupport> bundleToRcsMap = new ConcurrentHashMap<Long, RegionConfigurationSupport>();

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentActivator.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentActivator.java?rev=1744827&r1=1744826&r2=1744827&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentActivator.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentActivator.java Sat May 21 04:46:03 2016
@@ -37,6 +37,10 @@ public interface ComponentActivator exte
 
     void unregisterComponentId(AbstractComponentManager<?> sAbstractComponentManager);
 
+    <T> boolean enterCreate(ServiceReference<T> reference);
+
+    <T> void leaveCreate(ServiceReference<T> reference);
+
     <S, T> void registerMissingDependency(DependencyManager<S, T> dependencyManager,
                                               ServiceReference<T> serviceReference, int trackingCount);
 

Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java?rev=1744827&r1=1744826&r2=1744827&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/SingleComponentManager.java Sat May 21 04:46:03 2016
@@ -67,8 +67,6 @@ public class SingleComponentManager<S> e
     // null if properties are not to be overwritten
     private Dictionary<String, Object> m_serviceProperties;
 
-    private final ThreadLocal<Boolean> m_circularReferences = new ThreadLocal<Boolean>();
-
    /**
      * The constructor receives both the activator and the metadata
      * @param componentMethods
@@ -315,7 +313,6 @@ public class SingleComponentManager<S> e
         else
         {
             componentContext.setImplementationAccessible( true );
-            m_circularReferences.remove();
             //this may cause a getService as properties now match a filter.
             setServiceProperties( result );
         }
@@ -835,13 +832,10 @@ public class SingleComponentManager<S> e
     @Override
     boolean getServiceInternal(ServiceRegistration<S> serviceRegistration)
     {
-        if (m_circularReferences.get() != null)
+        if ( serviceRegistration != null && getActivator().enterCreate(serviceRegistration.getReference()))
         {
-            log( LogService.LOG_ERROR,  "Circular reference detected, getService returning null", null );
-            dumpThreads();
             return false;
         }
-        m_circularReferences.set( Boolean.TRUE );
         try
         {
             boolean success = true;
@@ -850,15 +844,13 @@ public class SingleComponentManager<S> e
                 ComponentContextImpl<S> componentContext = new ComponentContextImpl<S>(this, this.getBundle(), serviceRegistration);
                 if ( collectDependencies(componentContext))
                 {
-                        log(
-                                LogService.LOG_DEBUG,
-                                "getService (single component manager) dependencies collected.",
-                                null );
+                    log( LogService.LOG_DEBUG,
+                        "getService (single component manager) dependencies collected.",
+                        null );
                 }
                 else
                 {
-                    log(
-                            LogService.LOG_INFO,
+                    log( LogService.LOG_INFO,
                             "Could not obtain all required dependencies, getService returning null",
                             null );
                     success = false;
@@ -889,8 +881,10 @@ public class SingleComponentManager<S> e
         }
         finally
         {
-            //normally this will have been done after object becomes accessible.  This is double-checking.
-            m_circularReferences.remove();
+            if (serviceRegistration != null)
+            {
+                getActivator().leaveCreate(serviceRegistration.getReference());
+            }
         }
     }
 

Modified: felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/CircularReferenceTest.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/CircularReferenceTest.java?rev=1744827&r1=1744826&r2=1744827&view=diff
==============================================================================
--- felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/CircularReferenceTest.java (original)
+++ felix/trunk/scr/src/test/java/org/apache/felix/scr/integration/CircularReferenceTest.java Sat May 21 04:46:03 2016
@@ -290,9 +290,9 @@ public class CircularReferenceTest exten
         A a = bundleContext.getService( serviceReferenceA );
         assertNotNull( a );
         assertEquals( 1, a.getBs().size());
-  //test currently does not show desired result.              
-//        assertEquals( 1, b.getAs().size() );
-//        assertNotNull( b.getAs().get(0) );
+        delay();
+        assertEquals( 1, b.getAs().size() );
+        assertNotNull( b.getAs().get(0) );
 
     }
 }