You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@felix.apache.org by gn...@apache.org on 2013/08/02 15:22:57 UTC
svn commit: r1509691 - in
/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl:
helper/BindMethod.java manager/DependencyManager.java manager/RefPair.java
Author: gnodet
Date: Fri Aug 2 13:22:57 2013
New Revision: 1509691
URL: http://svn.apache.org/r1509691
Log:
[FELIX-4189] DS should not hold any lock while calling bundleContext#getService and #ungetService
Modified:
felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.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/RefPair.java
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java?rev=1509691&r1=1509690&r2=1509691&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java Fri Aug 2 13:22:57 2013
@@ -578,7 +578,20 @@ public class BindMethod extends BaseMeth
"Could not get service from ref {0}", new Object[] {refPair.getRef()}, null );
return false;
}
- refPair.setServiceObject( service );
+ Object oldService;
+ synchronized ( refPair )
+ {
+ oldService = refPair.getServiceObject();
+ if (oldService == null)
+ {
+ refPair.setServiceObject(service);
+ }
+ }
+ if (oldService != null)
+ {
+ // Another thread got the service before, so unget our
+ context.ungetService( refPair.getRef() );
+ }
return true;
}
}
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=1509691&r1=1509690&r2=1509691&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 Fri Aug 2 13:22:57 2013
@@ -216,16 +216,21 @@ public class DependencyManager<S, T> imp
protected void ungetService( RefPair<T> ref )
{
+ Object service;
synchronized ( ref )
{
- if ( ref.getServiceObject() != null )
+ service = ref.getServiceObject();
+ if ( ref != null )
{
ref.setServiceObject( null );
- BundleContext bundleContext = m_componentManager.getBundleContext();
- if ( bundleContext != null )
- {
- bundleContext.ungetService( ref.getRef() );
- }
+ }
+ }
+ if ( service != null )
+ {
+ BundleContext bundleContext = m_componentManager.getBundleContext();
+ if ( bundleContext != null )
+ {
+ bundleContext.ungetService( ref.getRef() );
}
}
}
@@ -377,16 +382,13 @@ public class DependencyManager<S, T> imp
SortedMap<ServiceReference<T>, RefPair<T>> tracked = getTracker().getTracked( true, trackingCount );
for (RefPair<T> refPair: tracked.values())
{
- synchronized (refPair)
+ if (getServiceObject( m_bindMethods.getBind(), refPair ))
{
- if (getServiceObject( m_bindMethods.getBind(), refPair ))
- {
- success = true;
- }
- else
- {
- m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(), trackingCount.get() );
- }
+ success = true;
+ }
+ else
+ {
+ m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(), trackingCount.get() );
}
}
return success;
@@ -499,14 +501,11 @@ public class DependencyManager<S, T> imp
SortedMap<ServiceReference<T>, RefPair<T>> tracked = getTracker().getTracked( success || !getTracker().isEmpty(), trackingCount );
for (RefPair<T> refPair: tracked.values())
{
- synchronized (refPair)
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
+ if ( refPair.isFailed() )
{
- success |= getServiceObject( m_bindMethods.getBind(), refPair );
- if ( refPair.isFailed() )
- {
- m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(),
- trackingCount.get() );
- }
+ m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(),
+ trackingCount.get() );
}
}
return success;
@@ -605,10 +604,7 @@ public class DependencyManager<S, T> imp
//another thread is concurrently opening, and it got done already
for (RefPair<T> refPair: refs)
{
- synchronized (refPair)
- {
- success |= getServiceObject( m_bindMethods.getBind(), refPair );
- }
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
}
return success;
}
@@ -617,10 +613,7 @@ public class DependencyManager<S, T> imp
SortedMap<ServiceReference<T>, RefPair<T>> tracked = getTracker().getTracked( true, trackingCount );
for (RefPair<T> refPair: tracked.values())
{
- synchronized (refPair)
- {
- success |= getServiceObject( m_bindMethods.getBind(), refPair );
- }
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
refs.add(refPair) ;
}
if ( this.refs.compareAndSet( null, refs ) )
@@ -778,12 +771,9 @@ public class DependencyManager<S, T> imp
}
if ( nextRefPair != null )
{
- synchronized ( nextRefPair )
+ if ( !getServiceObject( m_bindMethods.getBind(), nextRefPair ) )
{
- if ( !getServiceObject( m_bindMethods.getBind(), nextRefPair ) )
- {
- //TODO error???
- }
+ //TODO error???
}
if ( !nextRefPair.isFailed() )
{
@@ -841,14 +831,11 @@ public class DependencyManager<S, T> imp
}
if (refPair != null)
{
- synchronized( refPair )
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
+ if ( refPair.isFailed() )
{
- success |= getServiceObject( m_bindMethods.getBind(), refPair );
- if ( refPair.isFailed() )
- {
- m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(),
- trackingCount.get() );
- }
+ m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(),
+ trackingCount.get() );
}
}
return success;
@@ -985,14 +972,11 @@ public class DependencyManager<S, T> imp
}
if ( refPair != null )
{
- synchronized ( refPair )
+ success |= getServiceObject( m_bindMethods.getBind(), refPair );
+ if ( refPair.isFailed() )
{
- success |= getServiceObject( m_bindMethods.getBind(), refPair );
- if ( refPair.isFailed() )
- {
- m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(),
- trackingCount.get() );
- }
+ m_componentManager.registerMissingDependency( DependencyManager.this, refPair.getRef(),
+ trackingCount.get() );
}
}
}
@@ -1536,18 +1520,15 @@ public class DependencyManager<S, T> imp
}
//TODO dynamic reluctant
RefPair<T> refPair = trackerRef.get().getService( ref );
- synchronized ( refPair )
+ if (refPair.getServiceObject() != null)
{
- if (refPair.getServiceObject() != null)
- {
- m_componentManager.log( LogService.LOG_DEBUG,
- "DependencyManager : late binding of service reference {1} skipped as service has already been located",
- new Object[] {ref}, null );
- //something else got the reference and may be binding it.
- return;
- }
- getServiceObject( m_bindMethods.getBind(), refPair );
+ m_componentManager.log( LogService.LOG_DEBUG,
+ "DependencyManager : late binding of service reference {1} skipped as service has already been located",
+ new Object[] {ref}, null );
+ //something else got the reference and may be binding it.
+ return;
}
+ getServiceObject( m_bindMethods.getBind(), refPair );
m_componentManager.invokeBindMethod( this, refPair, trackingCount );
}
Modified: felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java
URL: http://svn.apache.org/viewvc/felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java?rev=1509691&r1=1509690&r2=1509691&view=diff
==============================================================================
--- felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java (original)
+++ felix/trunk/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java Fri Aug 2 13:22:57 2013
@@ -42,12 +42,12 @@ public class RefPair<T>
return ref;
}
- public T getServiceObject()
+ public synchronized T getServiceObject()
{
return serviceObject;
}
- public void setServiceObject( T serviceObject )
+ public synchronized void setServiceObject( T serviceObject )
{
this.serviceObject = serviceObject;
if ( serviceObject != null)
@@ -56,12 +56,12 @@ public class RefPair<T>
}
}
- public void setFailed( )
+ public synchronized void setFailed( )
{
this.failed = true;
}
- public boolean isFailed()
+ public synchronized boolean isFailed()
{
return failed;
}