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 2013/10/29 00:08:15 UTC
svn commit: r1536550 - in /felix/trunk/scr: ./
src/main/java/org/apache/felix/scr/impl/manager/
Author: djencks
Date: Mon Oct 28 23:08:15 2013
New Revision: 1536550
URL: http://svn.apache.org/r1536550
Log:
FELIX-4297 fix timing hole while opening DependencyManager
Modified:
felix/trunk/scr/changelog.txt
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.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/EdgeInfo.java
Modified: felix/trunk/scr/changelog.txt
URL: http://svn.apache.org/viewvc/felix/trunk/scr/changelog.txt?rev=1536550&r1=1536549&r2=1536550&view=diff
==============================================================================
--- felix/trunk/scr/changelog.txt (original)
+++ felix/trunk/scr/changelog.txt Mon Oct 28 23:08:15 2013
@@ -45,6 +45,7 @@ Changes from 1.6.2 to 1.8
* [FELIX-4287] - [DS] NPE when calling ComponentInstance.dispose after bundle shut down
* [FELIX-4290] - [DS] Issue with factory components with required configuration
* [FELIX-4293] - [DS] logic error in handling configuration LOCATION_CHANGED event
+ * [FELIX-4297] - [DS] timing hole in opening a dependency manager
** Task
* [FELIX-3584] - [DS] Handle new LOCATION_CHANGED event
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java?rev=1536550&r1=1536549&r2=1536550&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/ComponentContextImpl.java Mon Oct 28 23:08:15 2013
@@ -60,6 +60,10 @@ public class ComponentContextImpl<S> imp
m_usingBundle = usingBundle;
m_implementationObject = implementationObject;
edgeInfos = new EdgeInfo[componentManager.getComponentMetadata().getDependencies().size()];
+ for (int i = 0; i< edgeInfos.length; i++)
+ {
+ edgeInfos[i] = new EdgeInfo();
+ }
}
void setImplementationAccessible(boolean implementationAccessible)
@@ -74,18 +78,9 @@ public class ComponentContextImpl<S> imp
EdgeInfo getEdgeInfo(DependencyManager<S, ?> dm)
{
int index = dm.getIndex();
- if (edgeInfos[index] == null)
- {
- edgeInfos[index] = new EdgeInfo();
- }
return edgeInfos[index];
}
- void clearEdgeInfos()
- {
- Arrays.fill( edgeInfos, null );
- }
-
protected SingleComponentManager<S> getComponentManager()
{
return m_componentManager;
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=1536550&r1=1536549&r2=1536550&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 Mon Oct 28 23:08:15 2013
@@ -1409,12 +1409,12 @@ public class DependencyManager<S, T> imp
boolean success = m_dependencyMetadata.isOptional();
AtomicInteger trackingCount = new AtomicInteger( );
Collection<RefPair<T>> refs;
- CountDownLatch openLatch = new CountDownLatch(1);
+ CountDownLatch openLatch;
synchronized ( m_tracker.tracked() )
{
refs = m_customizer.getRefs( trackingCount );
edgeInfo.setOpen( trackingCount.get() );
- edgeInfo.setOpenLatch( openLatch );
+ openLatch = edgeInfo.getOpenLatch( );
}
m_componentManager.log( LogService.LOG_DEBUG,
"For dependency {0}, optional: {1}; to bind: {2}",
@@ -1423,7 +1423,7 @@ public class DependencyManager<S, T> imp
{
if ( !refPair.isFailed() )
{
- if ( !invokeBindMethod( componentInstance, refPair, trackingCount.get(), edgeInfo ) )
+ if ( !doInvokeBindMethod( componentInstance, refPair ) )
{
m_componentManager.log( LogService.LOG_DEBUG,
"For dependency {0}, failed to invoke bind method on object {1}",
@@ -1451,12 +1451,12 @@ public class DependencyManager<S, T> imp
AtomicInteger trackingCount = new AtomicInteger();
Collection<RefPair<T>> refPairs;
- CountDownLatch latch = new CountDownLatch( 1 );
+ CountDownLatch latch;
synchronized ( m_tracker.tracked() )
{
refPairs = m_customizer.getRefs( trackingCount );
edgeInfo.setClose( trackingCount.get() );
- edgeInfo.setCloseLatch( latch );
+ latch = edgeInfo.getCloseLatch( );
}
m_componentManager.log( LogService.LOG_DEBUG,
@@ -1534,6 +1534,7 @@ public class DependencyManager<S, T> imp
// null. This is valid for both immediate and delayed components
if ( componentInstance != null )
{
+ info.waitForOpen( m_componentManager, getName(), "invokeBindMethod" );
synchronized ( m_tracker.tracked() )
{
if (info.outOfRange( trackingCount ) )
@@ -1542,20 +1543,8 @@ public class DependencyManager<S, T> imp
return true;
}
}
- MethodResult result = m_bindMethods.getBind().invoke( componentInstance, refPair, MethodResult.VOID, m_componentManager );
- if ( result == null )
- {
- return false;
- }
- m_componentManager.setServiceProperties( result );
- return true;
+ return doInvokeBindMethod( componentInstance, refPair );
- // Concurrency Issue: The component instance still exists but
- // but the defined bind method field is null, fail binding
-// m_componentManager.log( LogService.LOG_INFO,
-// "DependencyManager : Component instance present, but DependencyManager shut down (no bind method)",
-// null );
-// return false;
}
else
{
@@ -1567,6 +1556,17 @@ public class DependencyManager<S, T> imp
}
}
+ private boolean doInvokeBindMethod(S componentInstance, RefPair refPair)
+ {
+ MethodResult result = m_bindMethods.getBind().invoke( componentInstance, refPair, MethodResult.VOID, m_componentManager );
+ if ( result == null )
+ {
+ return false;
+ }
+ m_componentManager.setServiceProperties( result );
+ return true;
+ }
+
/**
* Calls the updated method.
@@ -1585,6 +1585,7 @@ public class DependencyManager<S, T> imp
// null. This is valid for both immediate and delayed components
if ( componentInstance != null )
{
+ info.waitForOpen( m_componentManager, getName(), "invokeUpdatedMethod" );
synchronized ( m_tracker.tracked() )
{
if (info.outOfRange( trackingCount ) )
@@ -1593,34 +1594,6 @@ public class DependencyManager<S, T> imp
return;
}
}
- try
- {
- if (!info.getOpenLatch().await( getLockTimeout(), TimeUnit.MILLISECONDS ))
- {
- m_componentManager.log( LogService.LOG_ERROR,
- "DependencyManager : invokeUpdatedMethod : timeout on open latch {0}", new Object[] {getName()}, null );
- m_componentManager.dumpThreads();
- }
- }
- catch ( InterruptedException e )
- {
- try
- {
- if (!info.getOpenLatch().await( getLockTimeout(), TimeUnit.MILLISECONDS ))
- {
- m_componentManager.log( LogService.LOG_ERROR,
- "DependencyManager : invokeUpdatedMethod : timeout on open latch {0}", new Object[] {getName()}, null );
- m_componentManager.dumpThreads();
- }
- }
- catch ( InterruptedException e1 )
- {
- m_componentManager.log( LogService.LOG_ERROR,
- "DependencyManager : invokeUpdatedMethod : Interrupted twice on open latch {0}", new Object[] {getName()}, null );
- Thread.currentThread().interrupt();
- }
- Thread.currentThread().interrupt();
- }
if ( !getServiceObject( m_bindMethods.getUpdated(), refPair ))
{
m_componentManager.log( LogService.LOG_WARNING,
@@ -1664,62 +1637,23 @@ public class DependencyManager<S, T> imp
// null. This is valid for both immediate and delayed components
if ( componentInstance != null )
{
+ info.waitForOpen( m_componentManager, getName(), "invokeUnbindMethod" );
boolean outOfRange;
synchronized ( m_tracker.tracked() )
{
- outOfRange = info.outOfRange( trackingCount );
+ if (info.beforeRange( trackingCount ))
+ {
+ return;
+ }
+ outOfRange = info.afterRange( trackingCount );
}
if ( outOfRange )
{
//wait for unbinds to complete
- if (info.getCloseLatch() != null)
- {
- try
- {
- if (!info.getCloseLatch().await( getLockTimeout(), TimeUnit.MILLISECONDS ) )
- {
- m_componentManager.log( LogService.LOG_ERROR,
- "DependencyManager : invokeUnbindMethod : timeout on close latch {0}", new Object[] {getName()}, null );
- m_componentManager.dumpThreads();
- }
- }
- catch ( InterruptedException e )
- {
- try
- {
- if (!info.getCloseLatch().await( getLockTimeout(), TimeUnit.MILLISECONDS ) )
- {
- m_componentManager.log( LogService.LOG_ERROR,
- "DependencyManager : invokeUnbindMethod : timeout on close latch {0}", new Object[] {getName()}, null );
- m_componentManager.dumpThreads();
- }
- }
- catch ( InterruptedException e1 )
- {
- m_componentManager.log( LogService.LOG_ERROR,
- "DependencyManager : invokeUnbindMethod : Interrupted twice on close latch {0}", new Object[] {getName()}, null );
- }
- Thread.currentThread().interrupt();
- }
- }
+ info.waitForClose( m_componentManager, getName(), "invokeUnbindMethod" );
//ignore events after close started or we will have duplicate unbinds.
return;
}
- try
- {
- if (!info.getOpenLatch().await( getLockTimeout(), TimeUnit.MILLISECONDS ))
- {
- m_componentManager.log( LogService.LOG_WARNING,
- "DependencyManager : invokeUnbindMethod : timeout on open latch {0}", new Object[] {getName()}, null );
- m_componentManager.dumpThreads();
- }
- }
- catch ( InterruptedException e )
- {
- Thread.currentThread().interrupt();
- m_componentManager.dumpThreads();
- //ignore
- }
if ( !getServiceObject( m_bindMethods.getUnbind(), refPair ))
{
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/EdgeInfo.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/EdgeInfo.java?rev=1536550&r1=1536549&r2=1536550&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/EdgeInfo.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/EdgeInfo.java Mon Oct 28 23:08:15 2013
@@ -19,6 +19,9 @@
package org.apache.felix.scr.impl.manager;
import java.util.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.osgi.service.log.LogService;
/**
* EdgeInfo holds information about the service event tracking counts for creating (open) and disposing (close)
@@ -47,8 +50,8 @@ class EdgeInfo
{
private int open = -1;
private int close = -1;
- private CountDownLatch openLatch;
- private CountDownLatch closeLatch;
+ private final CountDownLatch openLatch = new CountDownLatch(1);
+ private final CountDownLatch closeLatch = new CountDownLatch(1);
public void setClose( int close )
{
@@ -59,20 +62,59 @@ class EdgeInfo
{
return openLatch;
}
-
- public void setOpenLatch( CountDownLatch latch )
- {
- this.openLatch = latch;
- }
- public CountDownLatch getCloseLatch()
+ public void waitForOpen(AbstractComponentManager m_componentManager, String componentName, String methodName)
{
- return closeLatch;
+
+ CountDownLatch latch = getOpenLatch();
+ String latchName = "open";
+ waitForLatch( m_componentManager, latch, componentName, methodName, latchName );
+ }
+
+ public void waitForClose(AbstractComponentManager m_componentManager, String componentName, String methodName)
+ {
+
+ CountDownLatch latch = getCloseLatch();
+ String latchName = "close";
+ waitForLatch( m_componentManager, latch, componentName, methodName, latchName );
+ }
+
+ private void waitForLatch(AbstractComponentManager m_componentManager, CountDownLatch latch, String componentName,
+ String methodName, String latchName)
+ {
+ try
+ {
+ if (!latch.await( m_componentManager.getLockTimeout(), TimeUnit.MILLISECONDS ))
+ {
+ m_componentManager.log( LogService.LOG_ERROR,
+ "DependencyManager : {0} : timeout on {1} latch {2}", new Object[] {methodName, latchName, componentName}, null );
+ m_componentManager.dumpThreads();
+ }
+ }
+ catch ( InterruptedException e )
+ {
+ try
+ {
+ if (!latch.await( m_componentManager.getLockTimeout(), TimeUnit.MILLISECONDS ))
+ {
+ m_componentManager.log( LogService.LOG_ERROR,
+ "DependencyManager : {0} : timeout on {1} latch {2}", new Object[] {methodName, latchName, componentName}, null );
+ m_componentManager.dumpThreads();
+ }
+ }
+ catch ( InterruptedException e1 )
+ {
+ m_componentManager.log( LogService.LOG_ERROR,
+ "DependencyManager : {0} : Interrupted twice on {1} latch {2}", new Object[] {methodName, latchName, componentName}, null );
+ Thread.currentThread().interrupt();
+ }
+ Thread.currentThread().interrupt();
+ }
}
- public void setCloseLatch( CountDownLatch latch )
+ public CountDownLatch getCloseLatch()
{
- this.closeLatch = latch;
+ return closeLatch;
}
public void setOpen( int open )
@@ -85,4 +127,18 @@ class EdgeInfo
return (open != -1 && trackingCount < open)
|| (close != -1 && trackingCount > close);
}
+
+ public boolean beforeRange( int trackingCount )
+ {
+ if (open == -1)
+ {
+ throw new IllegalStateException("beforeRange called before open range set");
+ }
+ return trackingCount < open;
+ }
+
+ public boolean afterRange( int trackingCount )
+ {
+ return close != -1 && trackingCount > close;
+ }
}
\ No newline at end of file