Fixes from Marc review.

4 files changed, 61 insertions, 57 deletions

diff --git a/java/src/Ice/ObjectPrxHelper.java b/java/src/Ice/ObjectPrxHelper.java
index 8934f0ba70c..6be7a09f3bb 100644
--- a/java/src/Ice/ObjectPrxHelper.java
+++ b/java/src/Ice/ObjectPrxHelper.java
@@ -495,10 +495,16 @@ public class ObjectPrxHelper implements ObjectPrx
         }
     }
 
-    public final synchronized int
+    public final int
     __handleException(LocalException ex, int cnt)
     {
-        _delegate = null;
+	//
+	// Only _delegate needs to be mutex protected here.
+	//
+	synchronized(this)
+	{
+	    _delegate = null;
+	}
 
         try
         {
@@ -553,8 +559,7 @@ public class ObjectPrxHelper implements ObjectPrx
 	if(cnt > 0)
 	{
 	    //
-	    // Sleep before retrying. TODO: is it safe to sleep here
-	    // with the mutex locked?
+	    // Sleep before retrying.
 	    //
 	    try
 	    {
@@ -587,6 +592,11 @@ public class ObjectPrxHelper implements ObjectPrx
             throw new LocationForwardIdentityException();
         }
 
+	//
+	// TODO: BENOIT: This is not thread-safe. Everywhere else in the
+	// code, _reference is considered immutable and is not mutex
+	// protected.
+	//
         _reference = _reference.changeAdapterId(h.__reference().adapterId);
         _reference = _reference.changeEndpoints(h.__reference().endpoints);
 
diff --git a/java/src/Ice/_ObjectDelM.java b/java/src/Ice/_ObjectDelM.java
index 4185d736ad4..0dd859195f1 100644
--- a/java/src/Ice/_ObjectDelM.java
+++ b/java/src/Ice/_ObjectDelM.java
@@ -250,31 +250,30 @@ public class _ObjectDelM implements _ObjectDel
 		}
 		catch (LocalException ex)
 		{
-		    if(cached.value)
+		    if(__reference.routerInfo == null && __reference.endpoints.length == 0)
 		    {
-			IceInternal.TraceLevels traceLevels = __reference.instance.traceLevels();
-			Logger logger = __reference.instance.logger();
-		    
-			if(traceLevels.retry >= 2)
+			assert(__reference.locatorInfo != null);
+			__reference.locatorInfo.clearCache(__reference);
+			
+			if(cached.value)
 			{
-			    String s = "connection to cached endpoints failed\n" +
-				       "removing endpoints from cache and trying one more time\n" + ex;
-			    logger.trace(traceLevels.retryCat, s);
+			    IceInternal.TraceLevels traceLevels = __reference.instance.traceLevels();
+			    Logger logger = __reference.instance.logger();
+			    
+			    if(traceLevels.retry >= 2)
+			    {
+				String s = "connection to cached endpoints failed\n" +
+				    "removing endpoints from cache and trying one more time\n" + ex;
+				logger.trace(traceLevels.retryCat, s);
+			    }
+			    
+			    continue;
 			}
-			
-			__reference.locatorInfo.removeEndpoints(__reference);
-			continue;
-		    }
-		    else
-		    {
-			throw ex;
 		    }
+
+		    throw ex;
 		}   
 
-		if(__reference.locatorInfo != null && !cached.value)
-		{
-		    __reference.locatorInfo.addEndpoints(__reference, endpoints);
-		}
 		break;
 	    }
 
diff --git a/java/src/IceInternal/LocatorAdapterTable.java b/java/src/IceInternal/LocatorAdapterTable.java
index 80367ad089c..7786164dc8e 100644
--- a/java/src/IceInternal/LocatorAdapterTable.java
+++ b/java/src/IceInternal/LocatorAdapterTable.java
@@ -28,11 +28,10 @@ public final class LocatorAdapterTable
 	_table.put(adapter, endpoints);
     }
 
-    synchronized void
+    synchronized IceInternal.Endpoint[]
     remove(String adapter)
     {
-	Object object = _table.remove(adapter);
-	assert(object != null);
+	return (IceInternal.Endpoint[])_table.remove(adapter);
     }
 
     private java.util.HashMap _table = new java.util.HashMap();
diff --git a/java/src/IceInternal/LocatorInfo.java b/java/src/IceInternal/LocatorInfo.java
index a4a87c2a21c..3df7de3648c 100644
--- a/java/src/IceInternal/LocatorInfo.java
+++ b/java/src/IceInternal/LocatorInfo.java
@@ -58,13 +58,16 @@ public final class LocatorInfo
     getEndpoints(Reference ref, Ice.BooleanHolder cached)
     {
 	IceInternal.Endpoint[] endpoints = null;
-	cached.value = false;
 
 	if(ref.adapterId.length() > 0)
 	{
+	    cached.value = true;
+	
 	    endpoints = _adapterTable.get(ref.adapterId);
 	    if(endpoints == null)
 	    {
+		cached.value = false;
+
 		//
 		// Search the adapter in the location service if we didn't
 		// find it in the cache.
@@ -84,24 +87,24 @@ public final class LocatorInfo
 		    // endpoints and raise a NoEndpointException().
 		    //
 		}
-	    }
-	    else
-	    {
-		cached.value = true;
+		
+		if(endpoints != null && endpoints.length > 0)
+		{
+		    _adapterTable.add(ref.adapterId, endpoints);
+		}
 	    }
 
-	    if(endpoints != null)
+	    if(endpoints != null && endpoints.length > 0)
 	    {
 		if(ref.instance.traceLevels().location >= 1)
 		{
 		    StringBuffer s = new StringBuffer();
 		    if(cached.value)
-			s.append("found object in local locator table\n");
+			s.append("found endpoints in locator table\n");
 		    else
-			s.append("found object in locator\n");
-		    s.append("identity = " + Ice.Util.identityToString(ref.identity) + "\n");
+			s.append("retrieved endpoints from locator, adding to locator table\n");
 		    s.append("adapter = " + ref.adapterId + "\n");
-		    s.append("endpoints = ");
+ 		    s.append("endpoints = ");
 		    final int sz = endpoints.length;
 		    for(int i = 0; i < sz; i++)
 		    {
@@ -118,33 +121,26 @@ public final class LocatorInfo
     }
 
     public void
-    addEndpoints(Reference ref, IceInternal.Endpoint[] endpoints)
-    {
-	assert(endpoints.length > 0);
-
-	if(ref.adapterId.length() > 0)
-	{
-	    _adapterTable.add(ref.adapterId, endpoints);
-
-	    if(ref.instance.traceLevels().location >= 2)
-	    {
-		String s = "added adapter to local locator table\nadapter = " + ref.adapterId;
-		ref.instance.logger().trace(ref.instance.traceLevels().locationCat, s);
-	    }
-	}
-    }
-
-    public void
-    removeEndpoints(Reference ref)
+    clearCache(Reference ref)
     {
 	if(ref.adapterId.length() > 0)
 	{
-	    _adapterTable.remove(ref.adapterId);
+	    IceInternal.Endpoint[] endpoints = _adapterTable.remove(ref.adapterId);
 
 	    if(ref.instance.traceLevels().location >= 2)
 	    {
-		String s = "removed adapter from local locator table\nadapter = " + ref.adapterId;
-		ref.instance.logger().trace(ref.instance.traceLevels().locationCat, s);
+		StringBuffer s = new StringBuffer();
+		s.append("removed endpoints from locator table\n");
+		s.append("adapter = " + ref.adapterId + "\n");
+		s.append("endpoints = ");
+		final int sz = endpoints.length;
+		for(int i = 0; i < sz; i++)
+		{
+		    s.append(endpoints[i].toString());
+		    if(i + 1 < sz)
+			s.append(":");
+		}
+		ref.instance.logger().trace(ref.instance.traceLevels().locationCat, s.toString());
 	    }
 	}
     }