fix for bug 1107: Glacier2/router test failure under JDK 1.4

6 files changed, 130 insertions, 2 deletions

diff --git a/java/src/Ice/ConnectionI.java b/java/src/Ice/ConnectionI.java
index 538950f53a8..ee57f415c01 100644
--- a/java/src/Ice/ConnectionI.java
+++ b/java/src/Ice/ConnectionI.java
@@ -184,6 +184,15 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 		    IceInternal.TraceUtil.traceHeader("received validate connection", is, _logger, _traceLevels);
 		}
 	    }
+	    catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+	    {
+		synchronized(this)
+		{
+		    setState(StateClosed, ex.get());
+		    assert(_exception != null);
+		    throw _exception;
+		}
+	    }
 	    catch(LocalException ex)
 	    {
 		synchronized(this)
@@ -576,6 +585,44 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 		_transceiver.write(stream, _endpoint.timeout());
 	    }
 	}
+	catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+	{
+	    synchronized(this)
+	    {
+		setState(StateClosed, ex.get());
+		assert(_exception != null);
+		
+		if(out != null)
+		{
+		    //
+		    // If the request has already been removed from
+		    // the request map, we are out of luck. It would
+		    // mean that finished() has been called already,
+		    // and therefore the exception has been set using
+		    // the Outgoing::finished() callback. In this
+		    // case, we cannot throw the exception here,
+		    // because we must not both raise an exception and
+		    // have Outgoing::finished() called with an
+		    // exception. This means that in some rare cases,
+		    // a request will not be retried even though it
+		    // could. But I honestly don't know how I could
+		    // avoid this, without a very elaborate and
+		    // complex design, which would be bad for
+		    // performance.
+		    //
+		    IceInternal.Outgoing o = (IceInternal.Outgoing)_requests.remove(requestId);
+		    if(o != null)
+		    {
+			assert(o == out);
+			throw new IceInternal.LocalExceptionWrapper(_exception, ex.retry());
+		    }
+		}
+		else
+		{
+		    throw new IceInternal.LocalExceptionWrapper(_exception, ex.retry());
+		}
+	    }
+	}
 	catch(LocalException ex)
 	{
 	    synchronized(this)
@@ -686,6 +733,36 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 		_transceiver.write(stream, _endpoint.timeout());
 	    }
 	}
+	catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+	{
+	    synchronized(this)
+	    {
+		setState(StateClosed, ex.get());
+		assert(_exception != null);
+		
+		//
+		// If the request has already been removed from the
+		// async request map, we are out of luck. It would
+		// mean that finished() has been called already, and
+		// therefore the exception has been set using the
+		// OutgoingAsync::__finished() callback. In this case,
+		// we cannot throw the exception here, because we must
+		// not both raise an exception and have
+		// OutgoingAsync::__finished() called with an
+		// exception. This means that in some rare cases, a
+		// request will not be retried even though it
+		// could. But I honestly don't know how I could avoid
+		// this, without a very elaborate and complex design,
+		// which would be bad for performance.
+		//
+		IceInternal.OutgoingAsync o = (IceInternal.OutgoingAsync)_asyncRequests.remove(requestId);
+		if(o != null)
+		{
+		    assert(o == out);
+		    throw new IceInternal.LocalExceptionWrapper(_exception, ex.retry());
+		}
+	    }
+	}
 	catch(LocalException ex)
 	{
 	    synchronized(this)
@@ -882,6 +959,20 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 		_transceiver.write(stream, _endpoint.timeout());
 	    }
 	}
+	catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+	{
+	    synchronized(this)
+	    {
+		setState(StateClosed, ex.get());
+		assert(_exception != null);
+		
+		//
+		// Since batch requests are all oneways (or datagrams), we
+		// must report the exception to the caller.
+		//
+		throw _exception;
+	    }
+	}
 	catch(LocalException ex)
 	{
 	    synchronized(this)
@@ -933,6 +1024,13 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 		_transceiver.write(stream, _endpoint.timeout());
 	    }
 	}
+	catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+	{
+	    synchronized(this)
+	    {
+		setState(StateClosed, ex.get());
+	    }
+	}
 	catch(LocalException ex)
 	{
 	    synchronized(this)
@@ -962,6 +1060,10 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 		    _acmAbsoluteTimeoutMillis = System.currentTimeMillis() + _acmTimeout * 1000;
 		}
 	    }
+	    catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+	    {
+		setState(StateClosed, ex.get());
+	    }
 	    catch(LocalException ex)
 	    {
 		setState(StateClosed, ex);
@@ -986,6 +1088,10 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 		initiateShutdown();
 	    }
 	}
+	catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+	{
+	    setState(StateClosed, ex.get());
+	}
 	catch(LocalException ex)
 	{
 	    setState(StateClosed, ex);
@@ -1620,6 +1726,10 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
             {
                 initiateShutdown();
             }
+            catch(IceInternal.LocalExceptionWrapper ex) // Java-specific workaround in Transceiver.write().
+            {
+                setState(StateClosed, ex.get());
+            }
             catch(LocalException ex)
             {
                 setState(StateClosed, ex);
@@ -1629,6 +1739,7 @@ public final class ConnectionI extends IceInternal.EventHandler implements Conne
 
     private void
     initiateShutdown()
+	throws IceInternal.LocalExceptionWrapper // Java-specific workaround in Transceiver.write().
     {
 	assert(_state == StateClosing);
 	assert(_dispatchCount == 0);
diff --git a/java/src/IceInternal/TcpTransceiver.java b/java/src/IceInternal/TcpTransceiver.java
index e2e7418d22f..831c36bdd3b 100644
--- a/java/src/IceInternal/TcpTransceiver.java
+++ b/java/src/IceInternal/TcpTransceiver.java
@@ -138,6 +138,7 @@ final class TcpTransceiver implements Transceiver
 
     public void
     write(BasicStream stream, int timeout)
+	throws LocalExceptionWrapper
     {
         java.nio.ByteBuffer buf = stream.prepareWrite();
 
diff --git a/java/src/IceInternal/Transceiver.java b/java/src/IceInternal/Transceiver.java
index 4934b6cdfe0..31369b2076a 100644
--- a/java/src/IceInternal/Transceiver.java
+++ b/java/src/IceInternal/Transceiver.java
@@ -15,7 +15,14 @@ public interface Transceiver
     void close();
     void shutdownWrite();
     void shutdownReadWrite();
-    void write(BasicStream stream, int timeout);
+    //
+    // NOTE: In Java, write() can raise LocalExceptionWrapper to indicate that
+    //       retrying may not be safe, which is necessary to address an issue
+    //       in the IceSSL implementation for JDK 1.4. We can remove this if
+    //	     we ever drop support for JDK 1.4 (also see Ice.ConnectionI).
+    //
+    void write(BasicStream stream, int timeout)
+	throws LocalExceptionWrapper;
     //
     // NOTE: In Java, read() returns a boolean to indicate whether the transceiver
     //       has read more data than requested.
diff --git a/java/src/IceInternal/UdpTransceiver.java b/java/src/IceInternal/UdpTransceiver.java
index 335a23329ec..ba4662348d3 100644
--- a/java/src/IceInternal/UdpTransceiver.java
+++ b/java/src/IceInternal/UdpTransceiver.java
@@ -67,6 +67,7 @@ final class UdpTransceiver implements Transceiver
 
     public void
     write(BasicStream stream, int timeout) // NOTE: timeout is not used
+	throws LocalExceptionWrapper
     {
         java.nio.ByteBuffer buf = stream.prepareWrite();
 
diff --git a/java/ssl/jdk1.4/IceSSL/TransceiverI.java b/java/ssl/jdk1.4/IceSSL/TransceiverI.java
index dfd207532cd..b72716942ba 100644
--- a/java/ssl/jdk1.4/IceSSL/TransceiverI.java
+++ b/java/ssl/jdk1.4/IceSSL/TransceiverI.java
@@ -116,6 +116,7 @@ final class TransceiverI implements IceInternal.Transceiver
 
     public void
     write(IceInternal.BasicStream stream, int timeout)
+	throws IceInternal.LocalExceptionWrapper
     {
 	java.nio.ByteBuffer buf = stream.prepareWrite();
 
@@ -181,9 +182,15 @@ final class TransceiverI implements IceInternal.Transceiver
 	    {
 		if(IceInternal.Network.connectionLost(ex))
 		{
+		    //
+		    // Java's SSL implementation might have successfully sent the
+		    // packet but then detected loss of connection and raised an
+		    // exception. As a result, we cannot be sure that it is safe
+		    // to retry in this situation, so we raise LocalExceptionWrapper.
+		    //
 		    Ice.ConnectionLostException se = new Ice.ConnectionLostException();
 		    se.initCause(ex);
-		    throw se;
+		    throw new IceInternal.LocalExceptionWrapper(se, false);
 		}
 		
 		Ice.SocketException se = new Ice.SocketException();
diff --git a/java/ssl/jdk1.5/IceSSL/TransceiverI.java b/java/ssl/jdk1.5/IceSSL/TransceiverI.java
index f6b4ea67c24..4dd6b019ac9 100644
--- a/java/ssl/jdk1.5/IceSSL/TransceiverI.java
+++ b/java/ssl/jdk1.5/IceSSL/TransceiverI.java
@@ -170,6 +170,7 @@ final class TransceiverI implements IceInternal.Transceiver
     //
     public synchronized void
     write(IceInternal.BasicStream stream, int timeout)
+	throws IceInternal.LocalExceptionWrapper
     {
 	//
 	// Complete handshaking first if necessary.