assertions are now correctly caught

13 files changed, 270 insertions, 54 deletions

diff --git a/java/CHANGES b/java/CHANGES
index 0170736dcd3..50c3fa28991 100644
--- a/java/CHANGES
+++ b/java/CHANGES
@@ -1,6 +1,9 @@
 Changes since version 1.3.0
 ---------------------------
 
+- Assertions in servant methods are now correctly caught and an error
+  message is printed using the logger.
+
 - Facets have been significantly redesigned.
 
   * Facets are no longer hierarchical. As a result, FacetPath (which
diff --git a/java/src/IceInternal/Connection.java b/java/src/IceInternal/Connection.java
index b43451aba7e..c0f728479ca 100644
--- a/java/src/IceInternal/Connection.java
+++ b/java/src/IceInternal/Connection.java
@@ -1093,7 +1093,7 @@ public final class Connection extends EventHandler
 	Incoming in = null;
 	try
 	{
-	    while(invoke-- > 0)
+	    while(invoke > 0)
 	    {
 		
 		//
@@ -1110,7 +1110,7 @@ public final class Connection extends EventHandler
 		//
 		if(response)
 		{
-		    assert(invoke == 0); // No further invocations if a response is expected.
+		    assert(invoke == 1); // No further invocations if a response is expected.
 		    os.writeBlob(_replyHdr);
 		    
 		    //
@@ -1124,7 +1124,7 @@ public final class Connection extends EventHandler
 		//
 		// If there are more invocations, we need the stream back.
 		//
-		if(invoke > 0)
+		if(--invoke > 0)
 		{
 		    stream.swap(is);
                 }
@@ -1135,6 +1135,8 @@ public final class Connection extends EventHandler
 	}
 	catch(Ice.LocalException ex)
 	{
+	    assert(invoke > 0);
+
 	    synchronized(this)
 	    {
 		setState(StateClosed, ex);
@@ -1142,15 +1144,23 @@ public final class Connection extends EventHandler
 	}
 	catch(AssertionError ex)
 	{
+	    assert(invoke > 0);
+
 	    //
 	    // Java only: Upon an assertion, we don't kill the whole
 	    // process, but just print the stack trace and close the
 	    // connection.
 	    //
-	    ex.printStackTrace();
 	    synchronized(this)
 	    {
-		setState(StateClosed, new Ice.UnknownException());
+		java.io.StringWriter sw = new java.io.StringWriter();
+		java.io.PrintWriter pw = new java.io.PrintWriter(sw);
+		ex.printStackTrace(pw);
+		pw.flush();
+		Ice.UnknownException exc = new Ice.UnknownException();
+		exc.unknown = sw.toString();
+		_logger.error(exc.unknown);
+		setState(StateClosed, exc);
 	    }
 	}
 	finally
@@ -1159,6 +1169,25 @@ public final class Connection extends EventHandler
 	    {
 		reclaimIncoming(in);
 	    }
+
+	    //
+	    // If invoke() above raised an exception, and therefore
+	    // neither sendResponse() nor sendNoResponse() has been
+	    // called, then we must decrement _dispatchCount here.
+	    //
+	    if(invoke > 0)
+	    {
+		synchronized(this)
+		{
+		    assert(_dispatchCount > 0);
+		    _dispatchCount -= invoke;
+		    assert(_dispatchCount >= 0);
+		    if(_dispatchCount == 0)
+		    {
+			notifyAll();
+		    }
+		}
+	    }
 	}
     }
 
diff --git a/java/src/IceInternal/Incoming.java b/java/src/IceInternal/Incoming.java
index cf0d5def6f7..a53d45c2c08 100644
--- a/java/src/IceInternal/Incoming.java
+++ b/java/src/IceInternal/Incoming.java
@@ -151,6 +151,8 @@ final public class Incoming extends IncomingBase
         }
         catch(Ice.RequestFailedException ex)
         {
+	    _is.endReadEncaps();
+
 	    if(ex.id == null)
 	    {
 		ex.id = _current.id;
@@ -209,12 +211,17 @@ final public class Incoming extends IncomingBase
 		_os.writeString(ex.operation);
             }
 
+	    //
+	    // Must be called last, so that if an exception is raised,
+	    // this function is definitely *not* called.
+	    //
 	    __finishInvoke();
-	    _is.endReadEncaps();
 	    return;
         }
         catch(Ice.LocalException ex)
         {
+	    _is.endReadEncaps();
+
 	    if(_os.instance().properties().getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0)
 	    {
 		__warning(ex);
@@ -232,8 +239,11 @@ final public class Incoming extends IncomingBase
 		_os.writeString(sw.toString());
             }
 
+	    //
+	    // Must be called last, so that if an exception is raised,
+	    // this function is definitely *not* called.
+	    //
 	    __finishInvoke();
-	    _is.endReadEncaps();
 	    return;
         }
         /* Not possible in Java - UserExceptions are checked exceptions
@@ -244,6 +254,8 @@ final public class Incoming extends IncomingBase
 	*/
         catch(Exception ex)
         {
+	    _is.endReadEncaps();
+
 	    if(_os.instance().properties().getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0)
 	    {
 		__warning(ex);
@@ -261,8 +273,11 @@ final public class Incoming extends IncomingBase
 		_os.writeString(sw.toString());
             }
 	    
+	    //
+	    // Must be called last, so that if an exception is raised,
+	    // this function is definitely *not* called.
+	    //
 	    __finishInvoke();
-	    _is.endReadEncaps();
 	    return;
         }
 	
@@ -272,6 +287,8 @@ final public class Incoming extends IncomingBase
 	// the caller of this operation.
 	//
 
+	_is.endReadEncaps();
+
 	//
 	// DispatchAsync is "pseudo dispatch status", used internally
 	// only to indicate async dispatch.
@@ -283,7 +300,6 @@ final public class Incoming extends IncomingBase
 	    // here.  We do *not* call __finishInvoke(), because
 	    // the call is not finished yet.
 	    //
-	    _is.endReadEncaps();
 	    return;
 	}
 
@@ -326,8 +342,11 @@ final public class Incoming extends IncomingBase
 	    }
 	}
 
+	//
+	// Must be called last, so that if an exception is raised,
+	// this function is definitely *not* called.
+	//
 	__finishInvoke();
-	_is.endReadEncaps();
     }
 
     public BasicStream
diff --git a/java/src/IceInternal/IncomingAsync.java b/java/src/IceInternal/IncomingAsync.java
index 98f6e5f871d..b2822df23c4 100644
--- a/java/src/IceInternal/IncomingAsync.java
+++ b/java/src/IceInternal/IncomingAsync.java
@@ -62,6 +62,10 @@ public class IncomingAsync extends IncomingBase
 	    _os.pos(save);
 	}
 	
+	//
+	// Must be called last, so that if an exception is raised,
+	// this function is definitely *not* called.
+	//
 	__finishInvoke();
     }
 
@@ -196,6 +200,10 @@ public class IncomingAsync extends IncomingBase
             }
 	}
 
+	//
+	// Must be called last, so that if an exception is raised,
+	// this function is definitely *not* called.
+	//
 	__finishInvoke();
     }
 
diff --git a/java/test/Ice/exceptions/AllTests.java b/java/test/Ice/exceptions/AllTests.java
index 21d673c48dc..c13fd3e90ca 100644
--- a/java/test/Ice/exceptions/AllTests.java
+++ b/java/test/Ice/exceptions/AllTests.java
@@ -617,6 +617,40 @@ public class AllTests
 	private Callback callback = new Callback();
     }
 
+    private static class AMI_Thrower_throwAssertExceptionI extends AMI_Thrower_throwAssertException
+    {
+	public void
+	ice_response()
+	{
+	    test(false);
+	}
+
+	public void
+	ice_exception(Ice.LocalException exc)
+	{
+	    try
+	    {
+		throw exc;
+	    }
+	    catch(Ice.ConnectionLostException ex)
+	    {
+	    }
+	    catch(Exception ex)
+	    {
+		test(false);
+	    }
+	    callback.called();
+	}
+
+	public boolean
+	check()
+	{
+	    return callback.check();
+	}
+
+	private Callback callback = new Callback();
+    }
+
     private static class AMI_WrongOperation_noSuchOperationI extends AMI_WrongOperation_noSuchOperation
     {
 	public void
@@ -962,6 +996,32 @@ public class AllTests
 	    System.out.println("ok");
 	}
 	
+	if(thrower.supportsAssertException())
+	{
+	    System.out.print("testing assert in the server... ");
+	    System.out.flush();
+	    
+	    try
+	    {
+		thrower.throwAssertException();
+		test(false);
+	    }
+	    catch(java.lang.AssertionError ex)
+	    {
+		assert(collocated);
+	    }
+	    catch(Ice.ConnectionLostException ex)
+	    {
+		assert(!collocated);
+	    }
+	    catch(Exception ex)
+	    {
+		test(false);
+	    }
+	    
+	    System.out.println("ok");
+	}
+
 	System.out.print("catching object not exist exception... ");
 	System.out.flush();
 
@@ -1178,6 +1238,18 @@ public class AllTests
 		System.out.println("ok");
 	    }
 
+	    if(thrower.supportsAssertException())
+	    {
+		System.out.print("testing assert in the server with AMI... ");
+		System.out.flush();
+	    
+		AMI_Thrower_throwAssertExceptionI cb = new AMI_Thrower_throwAssertExceptionI();
+		thrower.throwAssertException_async(cb);
+		test(cb.check());
+	
+		System.out.println("ok");
+	    }
+
 	    System.out.print("catching object not exist exception with AMI... ");
 	    System.out.flush();
 
@@ -1241,7 +1313,6 @@ public class AllTests
 	    test(cb.check());
 	
 	    System.out.println("ok");
-
 	}
 
         return thrower;
diff --git a/java/test/Ice/exceptions/DummyLogger.java b/java/test/Ice/exceptions/DummyLogger.java
new file mode 100644
index 00000000000..c5a0887d5dd
--- /dev/null
+++ b/java/test/Ice/exceptions/DummyLogger.java
@@ -0,0 +1,31 @@
+// **********************************************************************
+//
+// Copyright (c) 2003
+// ZeroC, Inc.
+// Billerica, MA, USA
+//
+// All Rights Reserved.
+//
+// Ice is free software; you can redistribute it and/or modify it under
+// the terms of the GNU General Public License version 2 as published by
+// the Free Software Foundation.
+//
+// **********************************************************************
+
+public final class DummyLogger extends Ice.LocalObjectImpl implements Ice.Logger
+{
+    public void
+    trace(String category, String message)
+    {
+    }
+
+    public void
+    warning(String message)
+    {
+    }
+
+    public void
+    error(String message)
+    {
+    }
+}
diff --git a/java/test/Ice/exceptions/Server.java b/java/test/Ice/exceptions/Server.java
index b768ecfa37a..ec835514335 100644
--- a/java/test/Ice/exceptions/Server.java
+++ b/java/test/Ice/exceptions/Server.java
@@ -17,8 +17,15 @@ public class Server
     private static int
     run(String[] args, Ice.Communicator communicator)
     {
+	//
+	// For this test, we need a dummy logger, otherwise the
+	// assertion test will print an error message.
+	//
+	communicator.setLogger(new DummyLogger());
+
         Ice.Properties properties = communicator.getProperties();
-        properties.setProperty("Ice.Warn.Dispatch", "0");
+	// We don't need to disable warnings, because we have a dummy logger.
+        //properties.setProperty("Ice.Warn.Dispatch", "0");
         properties.setProperty("TestAdapter.Endpoints", "default -p 12345 -t 10000");
         Ice.ObjectAdapter adapter = communicator.createObjectAdapter("TestAdapter");
         Ice.Object object = new ThrowerI(adapter);
diff --git a/java/test/Ice/exceptions/Test.ice b/java/test/Ice/exceptions/Test.ice
index 095fa989cbc..2ac6482d3e7 100644
--- a/java/test/Ice/exceptions/Test.ice
+++ b/java/test/Ice/exceptions/Test.ice
@@ -41,23 +41,11 @@ exception D
     int dMem;
 };
 
-/*
- * Generates code that cannot be compiled in Java.
- *
-module Mod
-{
-    exception A extends :: A
-    {
-	int a2Mem;
-    };
-};
- */
-
-
 ["ami"] interface Thrower
 {
     void shutdown();
     bool supportsUndeclaredExceptions();
+    bool supportsAssertException();
 
     void throwAasA(int a) throws A;
     void throwAorDasAorD(int a) throws A, D;
@@ -67,16 +55,12 @@ module Mod
     void throwCasB(int a, int b, int c) throws B;
     void throwCasC(int a, int b, int c) throws C;
 
-    //
-    // Not supported in Java.
-    //
-    //void throwModA(int a, int a2) throws Mod::A;
-
     void throwUndeclaredA(int a);
     void throwUndeclaredB(int a, int b);
     void throwUndeclaredC(int a, int b, int c);
     void throwLocalException();
     void throwNonIceException();
+    void throwAssertException();
 };
 
 ["ami"] interface WrongOperation
diff --git a/java/test/Ice/exceptions/ThrowerI.java b/java/test/Ice/exceptions/ThrowerI.java
index 3d653906ebf..227df0d45ce 100644
--- a/java/test/Ice/exceptions/ThrowerI.java
+++ b/java/test/Ice/exceptions/ThrowerI.java
@@ -32,6 +32,12 @@ public final class ThrowerI extends _ThrowerDisp
         return false;
     }
 
+    public boolean
+    supportsAssertException(Ice.Current current)
+    {
+        return true;
+    }
+
     public void
     throwAasA(int a, Ice.Current current)
         throws A
@@ -103,36 +109,42 @@ public final class ThrowerI extends _ThrowerDisp
     }
 
     public void
-    throwLocalException(Ice.Current current)
+    throwUndeclaredA(int a, Ice.Current current)
     {
-        throw new Ice.TimeoutException();
+        // Not possible in Java.
+        throw new Ice.UnknownUserException();
     }
 
     public void
-    throwNonIceException(Ice.Current current)
+    throwUndeclaredB(int a, int b, Ice.Current current)
     {
-        throw new RuntimeException();
+        // Not possible in Java.
+        throw new Ice.UnknownUserException();
     }
 
     public void
-    throwUndeclaredA(int a, Ice.Current current)
+    throwUndeclaredC(int a, int b, int c, Ice.Current current)
     {
         // Not possible in Java.
         throw new Ice.UnknownUserException();
     }
 
     public void
-    throwUndeclaredB(int a, int b, Ice.Current current)
+    throwLocalException(Ice.Current current)
     {
-        // Not possible in Java.
-        throw new Ice.UnknownUserException();
+        throw new Ice.TimeoutException();
     }
 
     public void
-    throwUndeclaredC(int a, int b, int c, Ice.Current current)
+    throwNonIceException(Ice.Current current)
     {
-        // Not possible in Java.
-        throw new Ice.UnknownUserException();
+        throw new RuntimeException();
+    }
+
+    public void
+    throwAssertException(Ice.Current current)
+    {
+        throw new java.lang.AssertionError();
     }
 
     private Ice.ObjectAdapter _adapter;
diff --git a/java/test/Ice/exceptionsAMD/DummyLogger.java b/java/test/Ice/exceptionsAMD/DummyLogger.java
new file mode 100644
index 00000000000..c5a0887d5dd
--- /dev/null
+++ b/java/test/Ice/exceptionsAMD/DummyLogger.java
@@ -0,0 +1,31 @@
+// **********************************************************************
+//
+// Copyright (c) 2003
+// ZeroC, Inc.
+// Billerica, MA, USA
+//
+// All Rights Reserved.
+//
+// Ice is free software; you can redistribute it and/or modify it under
+// the terms of the GNU General Public License version 2 as published by
+// the Free Software Foundation.
+//
+// **********************************************************************
+
+public final class DummyLogger extends Ice.LocalObjectImpl implements Ice.Logger
+{
+    public void
+    trace(String category, String message)
+    {
+    }
+
+    public void
+    warning(String message)
+    {
+    }
+
+    public void
+    error(String message)
+    {
+    }
+}
diff --git a/java/test/Ice/exceptionsAMD/Server.java b/java/test/Ice/exceptionsAMD/Server.java
index b768ecfa37a..ec835514335 100644
--- a/java/test/Ice/exceptionsAMD/Server.java
+++ b/java/test/Ice/exceptionsAMD/Server.java
@@ -17,8 +17,15 @@ public class Server
     private static int
     run(String[] args, Ice.Communicator communicator)
     {
+	//
+	// For this test, we need a dummy logger, otherwise the
+	// assertion test will print an error message.
+	//
+	communicator.setLogger(new DummyLogger());
+
         Ice.Properties properties = communicator.getProperties();
-        properties.setProperty("Ice.Warn.Dispatch", "0");
+	// We don't need to disable warnings, because we have a dummy logger.
+        //properties.setProperty("Ice.Warn.Dispatch", "0");
         properties.setProperty("TestAdapter.Endpoints", "default -p 12345 -t 10000");
         Ice.ObjectAdapter adapter = communicator.createObjectAdapter("TestAdapter");
         Ice.Object object = new ThrowerI(adapter);
diff --git a/java/test/Ice/exceptionsAMD/TestAMD.ice b/java/test/Ice/exceptionsAMD/TestAMD.ice
index 462472050f7..a6e0c26ddd3 100644
--- a/java/test/Ice/exceptionsAMD/TestAMD.ice
+++ b/java/test/Ice/exceptionsAMD/TestAMD.ice
@@ -41,6 +41,7 @@ exception D
 {
     void shutdown();
     bool supportsUndeclaredExceptions();
+    bool supportsAssertException();
 
     void throwAasA(int a) throws A;
     void throwAorDasAorD(int a) throws A, D;
@@ -54,6 +55,7 @@ exception D
     void throwUndeclaredC(int a, int b, int c);
     void throwLocalException();
     void throwNonIceException();
+    void throwAssertException();
 };
 
 ["ami", "amd"] interface WrongOperation
diff --git a/java/test/Ice/exceptionsAMD/ThrowerI.java b/java/test/Ice/exceptionsAMD/ThrowerI.java
index 7e9ccfff246..d6dafa507ec 100644
--- a/java/test/Ice/exceptionsAMD/ThrowerI.java
+++ b/java/test/Ice/exceptionsAMD/ThrowerI.java
@@ -34,6 +34,12 @@ public final class ThrowerI extends _ThrowerDisp
     }
 
     public void
+    supportsAssertException_async(AMD_Thrower_supportsAssertException cb, Ice.Current current)
+    {
+        cb.ice_response(true);
+    }
+
+    public void
     throwAasA_async(AMD_Thrower_throwAasA cb, int a, Ice.Current current)
         throws A
     {
@@ -117,18 +123,6 @@ public final class ThrowerI extends _ThrowerDisp
     }
 
     public void
-    throwLocalException_async(AMD_Thrower_throwLocalException cb, Ice.Current current)
-    {
-        cb.ice_exception(new Ice.TimeoutException());
-    }
-
-    public void
-    throwNonIceException_async(AMD_Thrower_throwNonIceException cb, Ice.Current current)
-    {
-        throw new RuntimeException();
-    }
-
-    public void
     throwUndeclaredA_async(AMD_Thrower_throwUndeclaredA cb, int a, Ice.Current current)
     {
         A ex = new A();
@@ -155,5 +149,23 @@ public final class ThrowerI extends _ThrowerDisp
         cb.ice_exception(ex);
     }
 
+    public void
+    throwLocalException_async(AMD_Thrower_throwLocalException cb, Ice.Current current)
+    {
+        cb.ice_exception(new Ice.TimeoutException());
+    }
+
+    public void
+    throwNonIceException_async(AMD_Thrower_throwNonIceException cb, Ice.Current current)
+    {
+        throw new RuntimeException();
+    }
+
+    public void
+    throwAssertException_async(AMD_Thrower_throwAssertException cb, Ice.Current current)
+    {
+        throw new java.lang.AssertionError();
+    }
+
     private Ice.ObjectAdapter _adapter;
 }