[FELIX-4189] DS should not hold any lock while calling bundleContext#getService and #ungetService
git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1509691 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java b/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java
index b1f8438..196286d 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/helper/BindMethod.java
@@ -578,7 +578,20 @@
"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;
}
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
index be9e09c..260c0d7 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/DependencyManager.java
@@ -216,16 +216,21 @@
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 @@
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 @@
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 @@
//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 @@
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 @@
}
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 @@
}
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 @@
}
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 @@
}
//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 );
}
diff --git a/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java b/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java
index a767d1c..8c3c456 100644
--- a/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java
+++ b/scr/src/main/java/org/apache/felix/scr/impl/manager/RefPair.java
@@ -42,12 +42,12 @@
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 void setFailed( )
+ public synchronized void setFailed( )
{
this.failed = true;
}
- public boolean isFailed()
+ public synchronized boolean isFailed()
{
return failed;
}