FElIX-4189 use atomic reference in RefPair for service object

git-svn-id: https://svn.apache.org/repos/asf/felix/trunk@1515659 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 196286d..8ed77d0 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,16 +578,7 @@
                          "Could not get service from ref {0}", new Object[] {refPair.getRef()}, null );
                     return false;
                 }
-                Object oldService;
-                synchronized ( refPair )
-                {
-                    oldService = refPair.getServiceObject();
-                    if (oldService == null)
-                    {
-                        refPair.setServiceObject(service);
-                    }
-                }
-                if (oldService != null)
+                if (!refPair.setServiceObject(service))
                 {
                     // Another thread got the service before, so unget our
                     context.ungetService( refPair.getRef() );
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 260c0d7..628bd0e 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,15 +216,7 @@
 
         protected void ungetService( RefPair<T> ref )
         {
-            Object service;
-            synchronized ( ref )
-            {
-                service = ref.getServiceObject();
-                if ( ref != null )
-                {
-                    ref.setServiceObject( null );
-                }
-            }
+            Object service = ref.unsetServiceObject();
             if ( service != null )
             {
                 BundleContext bundleContext = m_componentManager.getBundleContext();
@@ -1317,11 +1309,11 @@
             //we don't know about this reference
             return null;
         }
-        if ( refPair.getServiceObject() != null )
-        {
-            return refPair.getServiceObject();
-        }
         T serviceObject;
+        if ( (serviceObject = refPair.getServiceObject()) != null )
+        {
+            return serviceObject;
+        }
         // otherwise acquire the service
         final BundleContext bundleContext = m_componentManager.getBundleContext();
         if (bundleContext == null)
@@ -1347,12 +1339,14 @@
         }
 
         // keep the service for later ungetting
-        if ( serviceObject != null )
+        if ( !refPair.setServiceObject( serviceObject ) )
         {
-            refPair.setServiceObject( serviceObject );
+            //another thread got the service first
+            bundleContext.ungetService( refPair.getRef() );
         }
 
         // return the acquired service (may be null of course)
+        //even if we did not set the service object, all the getService are for the same bundle so will have the same object.
         return serviceObject;
     }
 
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 8c3c456..cee9171 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
@@ -20,6 +20,8 @@
 
 package org.apache.felix.scr.impl.manager;
 
+import java.util.concurrent.atomic.AtomicReference;
+
 import org.osgi.framework.ServiceReference;
 
 /**
@@ -28,7 +30,7 @@
 public class RefPair<T>
 {
     private final ServiceReference<T> ref;
-    private T serviceObject;
+    private AtomicReference<T> serviceObjectRef = new AtomicReference<T>();
 
     private boolean failed;
 
@@ -42,26 +44,32 @@
         return ref;
     }
 
-    public synchronized T getServiceObject()
+    public T getServiceObject()
     {
-        return serviceObject;
+        return serviceObjectRef.get();
     }
 
-    public synchronized void setServiceObject( T serviceObject )
+    public boolean setServiceObject( T serviceObject )
     {
-        this.serviceObject = serviceObject;
+        boolean set = serviceObjectRef.compareAndSet( null, serviceObject );
         if ( serviceObject != null)
         {
             failed = false;
         }
+        return set;
+    }
+    
+    public T unsetServiceObject()
+    {
+        return serviceObjectRef.getAndSet( null );
     }
 
-    public synchronized void setFailed( )
+    public void setFailed( )
     {
         this.failed = true;
     }
 
-    public synchronized boolean isFailed()
+    public boolean isFailed()
     {
         return failed;
     }
@@ -70,6 +78,6 @@
     @Override
     public String toString()
     {
-        return "[RefPair: ref: [" + ref + "] service: [" + serviceObject + "]]";
+        return "[RefPair: ref: [" + ref + "] service: [" + serviceObjectRef.get() + "]]";
     }
 }