bug 4733 - fixing bugs in AMD exceptions

18 files changed, 245 insertions, 31 deletions

diff --git a/cpp/src/Ice/Incoming.cpp b/cpp/src/Ice/Incoming.cpp
index 294285218d5..07dcaa6da5d 100644
--- a/cpp/src/Ice/Incoming.cpp
+++ b/cpp/src/Ice/Incoming.cpp
@@ -34,7 +34,7 @@ extern bool ICE_DECLSPEC_IMPORT printStackTraces;
 
 }
 
-IceInternal::IncomingBase::IncomingBase(Instance* instance, ConnectionI* connection, 
+IceInternal::IncomingBase::IncomingBase(Instance* instance, ConnectionI* connection,
                                         const ObjectAdapterPtr& adapter,
                                         bool response, Byte compress, Int requestId) :
     _response(response),
@@ -60,21 +60,21 @@ IceInternal::IncomingBase::adopt(IncomingBase& other)
 {
     _servant = other._servant;
     other._servant = 0;
-    
+
     _locator = other._locator;
     other._locator = 0;
-    
+
     _cookie = other._cookie;
     other._cookie = 0;
-    
+
     _response = other._response;
     other._response = false;
-    
+
     _compress = other._compress;
     other._compress = 0;
-    
+
     _os.swap(other._os);
-    
+
     _connection = other._connection;
     other._connection = 0;
 }
@@ -94,7 +94,7 @@ void
 IceInternal::IncomingBase::__warning(const string& msg) const
 {
     Warning out(_os.instance()->initializationData().logger);
-    
+
     out << "dispatch exception: " << msg;
     out << "\nidentity: " << _os.instance()->identityToString(_current.id);
     out << "\nfacet: " << IceUtilInternal::escapeString(_current.facet, "");
@@ -112,6 +112,8 @@ IceInternal::IncomingBase::__servantLocatorFinished()
     }
     catch(const UserException& ex)
     {
+        assert(_connection);
+
         //
         // The operation may have already marshaled a reply; we must overwrite that reply.
         //
@@ -129,6 +131,8 @@ IceInternal::IncomingBase::__servantLocatorFinished()
         {
             _connection->sendNoResponse();
         }
+
+        _connection = 0;
     }
     catch(const std::exception& ex)
     {
@@ -144,6 +148,8 @@ IceInternal::IncomingBase::__servantLocatorFinished()
 void
 IceInternal::IncomingBase::__handleException(const std::exception& exc)
 {
+    assert(_connection);
+
     if(dynamic_cast<const RequestFailedException*>(&exc))
     {
         RequestFailedException* rfe =
@@ -153,12 +159,12 @@ IceInternal::IncomingBase::__handleException(const std::exception& exc)
         {
             rfe->id = _current.id;
         }
-        
+
         if(rfe->facet.empty() && !_current.facet.empty())
         {
             rfe->facet = _current.facet;
         }
-        
+
         if(rfe->operation.empty() && !_current.operation.empty())
         {
             rfe->operation = _current.operation;
@@ -205,7 +211,7 @@ IceInternal::IncomingBase::__handleException(const std::exception& exc)
             }
 
             _os.write(rfe->operation, false);
-            
+
             _connection->sendResponse(&_os, _compress);
         }
         else
@@ -213,9 +219,9 @@ IceInternal::IncomingBase::__handleException(const std::exception& exc)
             _connection->sendNoResponse();
         }
     }
-    else if(const Exception* ex = dynamic_cast<const Exception*>(&exc)) 
+    else if(const Exception* ex = dynamic_cast<const Exception*>(&exc))
     {
-        
+
         if(_os.instance()->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0)
         {
             __warning(*ex);
@@ -286,7 +292,7 @@ IceInternal::IncomingBase::__handleException(const std::exception& exc)
         {
             __warning(string("std::exception: ") + exc.what());
         }
-    
+
         if(_response)
         {
             _os.endWriteEncaps();
@@ -300,8 +306,10 @@ IceInternal::IncomingBase::__handleException(const std::exception& exc)
         else
         {
             _connection->sendNoResponse();
-        }    
+        }
     }
+
+    _connection = 0;
 }
 
 void
@@ -311,7 +319,9 @@ IceInternal::IncomingBase::__handleException()
     {
         __warning("unknown c++ exception");
     }
-    
+
+    assert(_connection);
+
     if(_response)
     {
         _os.endWriteEncaps();
@@ -325,10 +335,12 @@ IceInternal::IncomingBase::__handleException()
     {
         _connection->sendNoResponse();
     }
+
+    _connection = 0;
 }
 
 
-IceInternal::Incoming::Incoming(Instance* instance, ConnectionI* connection, 
+IceInternal::Incoming::Incoming(Instance* instance, ConnectionI* connection,
                                 const ObjectAdapterPtr& adapter,
                                 bool response, Byte compress, Int requestId) :
     IncomingBase(instance, connection, adapter, response, compress, requestId),
@@ -338,19 +350,19 @@ IceInternal::Incoming::Incoming(Instance* instance, ConnectionI* connection,
 }
 
 
-void 
+void
 IceInternal::Incoming::push(const Ice::DispatchInterceptorAsyncCallbackPtr& cb)
 {
     _interceptorAsyncCallbackQueue.push_front(cb);
 }
 
-void 
+void
 IceInternal::Incoming::pop()
 {
     _interceptorAsyncCallbackQueue.pop_front();
 }
 
-void 
+void
 IceInternal::Incoming::startOver()
 {
     if(_inParamPos == 0)
@@ -363,23 +375,23 @@ IceInternal::Incoming::startOver()
     else
     {
         killAsync();
-        
+
         //
         // Let's rewind _is and clean-up _os
         //
         _is.i = _inParamPos;
-        
+
         if(_response)
         {
             _os.endWriteEncaps();
-            _os.b.resize(headerSize + 4); 
+            _os.b.resize(headerSize + 4);
             _os.write(static_cast<Byte>(0));
             _os.startWriteEncaps();
         }
     }
 }
 
-void 
+void
 IceInternal::Incoming::killAsync()
 {
     //
@@ -566,18 +578,20 @@ IceInternal::Incoming::invoke(const ServantManagerPtr& servantManager)
         return;
     }
 
+    assert(_connection);
+
     if(_response)
     {
         _os.endWriteEncaps();
-        
+
         if(replyStatus != replyOK && replyStatus != replyUserException)
         {
             assert(replyStatus == replyObjectNotExist ||
                    replyStatus == replyFacetNotExist);
-            
+
             _os.b.resize(headerSize + 4); // Reply status position.
             _os.write(replyStatus);
-            
+
             _current.id.__write(&_os);
 
             //
@@ -605,6 +619,8 @@ IceInternal::Incoming::invoke(const ServantManagerPtr& servantManager)
     {
         _connection->sendNoResponse();
     }
+
+    _connection = 0;
 }
 
 
diff --git a/cpp/src/Ice/IncomingAsync.cpp b/cpp/src/Ice/IncomingAsync.cpp
index fa67937d0d0..124bd5553b5 100644
--- a/cpp/src/Ice/IncomingAsync.cpp
+++ b/cpp/src/Ice/IncomingAsync.cpp
@@ -119,7 +119,20 @@ IceInternal::IncomingAsync::ice_exception(const ::std::exception& ex)
         _active = false;
     }
 
-    __exception(ex);
+    if(_connection)
+    {
+        __exception(ex);
+    }
+    else
+    {
+        //
+        // Response has already been sent.
+        //
+        if(_os.instance()->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0)
+        {
+            __warning(ex.what());
+        }
+    }
 }
 
 void
@@ -157,7 +170,20 @@ IceInternal::IncomingAsync::ice_exception()
         _active = false;
     }
 
-    __exception();
+    if(_connection)
+    {
+        __exception();
+    }
+    else
+    {
+        //
+        // Response has already been sent.
+        //
+        if(_os.instance()->initializationData().properties->getPropertyAsIntWithDefault("Ice.Warn.Dispatch", 1) > 0)
+        {
+            __warning("unknown exception");
+        }
+    }
 }
 
 void
@@ -170,6 +196,8 @@ IceInternal::IncomingAsync::__response(bool ok)
             return;
         }
 
+        assert(_connection);
+
         if(_response)
         {
             _os.endWriteEncaps();
@@ -189,6 +217,8 @@ IceInternal::IncomingAsync::__response(bool ok)
         {
             _connection->sendNoResponse();
         }
+
+        _connection = 0;
     }
     catch(const LocalException& ex)
     {
@@ -310,4 +340,3 @@ IceAsync::Ice::AMD_Object_ice_invoke::ice_response(bool ok, const pair<const Byt
         __response(ok);
     }
 }
-
diff --git a/cpp/test/Ice/exceptions/AllTests.cpp b/cpp/test/Ice/exceptions/AllTests.cpp
index 2c8a36ad5a4..b89384a448a 100644
--- a/cpp/test/Ice/exceptions/AllTests.cpp
+++ b/cpp/test/Ice/exceptions/AllTests.cpp
@@ -1395,6 +1395,31 @@ allTests(const Ice::CommunicatorPtr& communicator, bool collocated)
 
     cout << "ok" << endl;
 
+    cout << "testing asynchronous exceptions... " << flush;
+
+    try
+    {
+        thrower->throwAfterResponse();
+    }
+    catch(...)
+    {
+        test(false);
+    }
+
+    try
+    {
+        thrower->throwAfterException();
+    }
+    catch(const A&)
+    {
+    }
+    catch(...)
+    {
+        test(false);
+    }
+
+    cout << "ok" << endl;
+
     if(!collocated)
     {
         cout << "catching exact types with AMI... " << flush;
diff --git a/cpp/test/Ice/exceptions/Test.ice b/cpp/test/Ice/exceptions/Test.ice
index b7c588f178a..d0887ac4264 100644
--- a/cpp/test/Ice/exceptions/Test.ice
+++ b/cpp/test/Ice/exceptions/Test.ice
@@ -70,6 +70,9 @@ module Mod
     void throwLocalException();
     void throwNonIceException();
     void throwAssertException();
+
+    void throwAfterResponse();
+    void throwAfterException() throws A;
 };
 
 ["ami"] interface WrongOperation
diff --git a/cpp/test/Ice/exceptions/TestAMD.ice b/cpp/test/Ice/exceptions/TestAMD.ice
index 84cdf1f91ea..3e51d8431fd 100644
--- a/cpp/test/Ice/exceptions/TestAMD.ice
+++ b/cpp/test/Ice/exceptions/TestAMD.ice
@@ -66,6 +66,9 @@ module Mod
     void throwLocalException();
     void throwNonIceException();
     void throwAssertException();
+
+    void throwAfterResponse();
+    void throwAfterException() throws A;
 };
 
 ["ami", "amd"] interface WrongOperation
diff --git a/cpp/test/Ice/exceptions/TestAMDI.cpp b/cpp/test/Ice/exceptions/TestAMDI.cpp
index e77a3a68bcf..503e33f8535 100644
--- a/cpp/test/Ice/exceptions/TestAMDI.cpp
+++ b/cpp/test/Ice/exceptions/TestAMDI.cpp
@@ -182,3 +182,19 @@ ThrowerI::throwAssertException_async(const AMD_Thrower_throwAssertExceptionPtr&,
 {
     assert(false); // Not supported in C++.
 }
+
+void
+ThrowerI::throwAfterResponse_async(const AMD_Thrower_throwAfterResponsePtr& cb, const Ice::Current&)
+{
+    cb->ice_response();
+
+    throw std::string();
+}
+
+void
+ThrowerI::throwAfterException_async(const AMD_Thrower_throwAfterExceptionPtr& cb, const Ice::Current&)
+{
+    cb->ice_exception(A());
+
+    throw std::string();
+}
diff --git a/cpp/test/Ice/exceptions/TestAMDI.h b/cpp/test/Ice/exceptions/TestAMDI.h
index 1b437a9123e..0ebf66d76a7 100644
--- a/cpp/test/Ice/exceptions/TestAMDI.h
+++ b/cpp/test/Ice/exceptions/TestAMDI.h
@@ -55,6 +55,9 @@ public:
                                             const Ice::Current&);
     virtual void throwAssertException_async(const Test::AMD_Thrower_throwAssertExceptionPtr&,
                                             const Ice::Current&);
+
+    virtual void throwAfterResponse_async(const Test::AMD_Thrower_throwAfterResponsePtr&, const Ice::Current&);
+    virtual void throwAfterException_async(const Test::AMD_Thrower_throwAfterExceptionPtr&, const Ice::Current&);
 };
 
 #endif
diff --git a/cpp/test/Ice/exceptions/TestI.cpp b/cpp/test/Ice/exceptions/TestI.cpp
index 88057292a21..17a9297b3bb 100644
--- a/cpp/test/Ice/exceptions/TestI.cpp
+++ b/cpp/test/Ice/exceptions/TestI.cpp
@@ -149,3 +149,20 @@ ThrowerI::throwAssertException(const Ice::Current&)
 {
     assert(false); // Not supported in C++.
 }
+
+void
+ThrowerI::throwAfterResponse(const Ice::Current&)
+{
+    //
+    // Only relevant for AMD.
+    //
+}
+
+void
+ThrowerI::throwAfterException(const Ice::Current&)
+{
+    //
+    // Only relevant for AMD.
+    //
+    throw A();
+}
diff --git a/cpp/test/Ice/exceptions/TestI.h b/cpp/test/Ice/exceptions/TestI.h
index c9f4af11065..825d2bdb743 100644
--- a/cpp/test/Ice/exceptions/TestI.h
+++ b/cpp/test/Ice/exceptions/TestI.h
@@ -38,6 +38,9 @@ public:
     virtual void throwLocalException(const Ice::Current&);
     virtual void throwNonIceException(const Ice::Current&);
     virtual void throwAssertException(const Ice::Current&);
+
+    virtual void throwAfterResponse(const Ice::Current&);
+    virtual void throwAfterException(const Ice::Current&);
 };
 
 #endif
diff --git a/cpp/test/Ice/servantLocator/AllTests.cpp b/cpp/test/Ice/servantLocator/AllTests.cpp
index c556bb56c16..5c67cddb052 100644
--- a/cpp/test/Ice/servantLocator/AllTests.cpp
+++ b/cpp/test/Ice/servantLocator/AllTests.cpp
@@ -296,6 +296,40 @@ allTests(const CommunicatorPtr& communicator, bool collocated)
     base = communicator->stringToProxy("category/finished:default -p 12010");
     obj = TestIntfPrx::checkedCast(base);
     testExceptions(obj, collocated);
+
+    //
+    // Only call these for category/finished.
+    //
+    try
+    {
+        obj->asyncResponse();
+    }
+    catch(const TestIntfUserException&)
+    {
+        test(false);
+    }
+    catch(const TestImpossibleException&)
+    {
+        //
+        // Called by finished().
+        //
+    }
+
+    try
+    {
+        obj->asyncException();
+    }
+    catch(const TestIntfUserException&)
+    {
+        test(false);
+    }
+    catch(const TestImpossibleException&)
+    {
+        //
+        // Called by finished().
+        //
+    }
+
     cout << "ok" << endl;
 
     cout << "testing servant locator removal... " << flush;
diff --git a/cpp/test/Ice/servantLocator/ServantLocatorI.cpp b/cpp/test/Ice/servantLocator/ServantLocatorI.cpp
index a00a2c5680e..d220ca81ecc 100644
--- a/cpp/test/Ice/servantLocator/ServantLocatorI.cpp
+++ b/cpp/test/Ice/servantLocator/ServantLocatorI.cpp
@@ -19,7 +19,8 @@ using namespace Test;
 
 ServantLocatorI::ServantLocatorI(const string& category) :
     _category(category),
-    _deactivated(false)
+    _deactivated(false),
+    _requestId(-1)
 {
 }
 
@@ -45,6 +46,12 @@ ServantLocatorI::locate(const Ice::Current& current, Ice::LocalObjectPtr& cookie
         exception(current);
     }
 
+    //
+    // Ensure locate() is only called once per request.
+    //
+    test(_requestId == -1);
+    _requestId = current.requestId;
+
     return newServantAndCookie(cookie);
 }
 
@@ -53,6 +60,13 @@ ServantLocatorI::finished(const Ice::Current& current, const Ice::ObjectPtr& ser
                           const Ice::LocalObjectPtr& cookie)
 {
     test(!_deactivated);
+
+    //
+    // Ensure finished() is only called once per request.
+    //
+    test(_requestId == current.requestId);
+    _requestId = -1;
+
     test(current.id.category == _category  || _category.empty());
     test(current.id.name == "locate" || current.id.name == "finished");
 
@@ -123,4 +137,12 @@ ServantLocatorI::exception(const Ice::Current& current)
     {
         throw TestImpossibleException(); // Yes, it really is meant to be TestImpossibleException.
     }
+    else if(current.operation == "asyncResponse")
+    {
+        throw TestImpossibleException();
+    }
+    else if(current.operation == "asyncException")
+    {
+        throw TestImpossibleException();
+    }
 }
diff --git a/cpp/test/Ice/servantLocator/ServantLocatorI.h b/cpp/test/Ice/servantLocator/ServantLocatorI.h
index 83ddc8e0362..7db29975ff8 100644
--- a/cpp/test/Ice/servantLocator/ServantLocatorI.h
+++ b/cpp/test/Ice/servantLocator/ServantLocatorI.h
@@ -38,6 +38,7 @@ private:
 
     const std::string _category;
     bool _deactivated;
+    Ice::Int _requestId;
 };
 
 };
diff --git a/cpp/test/Ice/servantLocator/Test.ice b/cpp/test/Ice/servantLocator/Test.ice
index 52c4c100609..50860085a02 100644
--- a/cpp/test/Ice/servantLocator/Test.ice
+++ b/cpp/test/Ice/servantLocator/Test.ice
@@ -37,6 +37,9 @@ interface TestIntf
     string impossibleException(bool throw) throws TestImpossibleException;
     string intfUserException(bool throw) throws TestIntfUserException, TestImpossibleException;
     
+    void asyncResponse() throws TestIntfUserException, TestImpossibleException;
+    void asyncException() throws TestIntfUserException, TestImpossibleException;
+
     void shutdown();
 };
 
diff --git a/cpp/test/Ice/servantLocator/TestAMD.ice b/cpp/test/Ice/servantLocator/TestAMD.ice
index 60de7bdb0cb..2f5d106e3dc 100644
--- a/cpp/test/Ice/servantLocator/TestAMD.ice
+++ b/cpp/test/Ice/servantLocator/TestAMD.ice
@@ -37,6 +37,9 @@ exception TestImpossibleException
     string impossibleException(bool throw) throws TestImpossibleException;
     string intfUserException(bool throw) throws TestIntfUserException, TestImpossibleException;
 
+    void asyncResponse() throws TestIntfUserException, TestImpossibleException;
+    void asyncException() throws TestIntfUserException, TestImpossibleException;
+
     void shutdown();
 };
 
diff --git a/cpp/test/Ice/servantLocator/TestAMDI.cpp b/cpp/test/Ice/servantLocator/TestAMDI.cpp
index 533c8447958..3ca3a0a7e93 100644
--- a/cpp/test/Ice/servantLocator/TestAMDI.cpp
+++ b/cpp/test/Ice/servantLocator/TestAMDI.cpp
@@ -104,6 +104,20 @@ TestAMDI::intfUserException_async(const Test::AMD_TestIntf_intfUserExceptionPtr&
 }
 
 void
+TestAMDI::asyncResponse_async(const Test::AMD_TestIntf_asyncResponsePtr& cb, const Current&)
+{
+    cb->ice_response();
+    throw Ice::ObjectNotExistException(__FILE__, __LINE__);
+}
+
+void
+TestAMDI::asyncException_async(const Test::AMD_TestIntf_asyncExceptionPtr& cb, const Current&)
+{
+    cb->ice_exception(Test::TestIntfUserException());
+    throw Ice::ObjectNotExistException(__FILE__, __LINE__);
+}
+
+void
 TestAMDI::shutdown_async(const Test::AMD_TestIntf_shutdownPtr& cb, const Current& current)
 {
     current.adapter->deactivate();
diff --git a/cpp/test/Ice/servantLocator/TestAMDI.h b/cpp/test/Ice/servantLocator/TestAMDI.h
index 42c84a65a96..9f8a6900d20 100644
--- a/cpp/test/Ice/servantLocator/TestAMDI.h
+++ b/cpp/test/Ice/servantLocator/TestAMDI.h
@@ -31,6 +31,9 @@ public:
     virtual void impossibleException_async(const Test::AMD_TestIntf_impossibleExceptionPtr&, bool, const Ice::Current&);
     virtual void intfUserException_async(const Test::AMD_TestIntf_intfUserExceptionPtr&, bool, const Ice::Current&);
 
+    virtual void asyncResponse_async(const Test::AMD_TestIntf_asyncResponsePtr&, const Ice::Current&);
+    virtual void asyncException_async(const Test::AMD_TestIntf_asyncExceptionPtr&, const Ice::Current&);
+
     virtual void shutdown_async(const Test::AMD_TestIntf_shutdownPtr&, const Ice::Current&);
 };
 
diff --git a/cpp/test/Ice/servantLocator/TestI.cpp b/cpp/test/Ice/servantLocator/TestI.cpp
index fc15bf537d4..611d19dde8a 100644
--- a/cpp/test/Ice/servantLocator/TestI.cpp
+++ b/cpp/test/Ice/servantLocator/TestI.cpp
@@ -89,6 +89,22 @@ TestI::intfUserException(bool _cpp_throw, const Current&)
 }
 
 void
+TestI::asyncResponse(const Current&)
+{
+    //
+    // Only relevant for AMD.
+    //
+}
+
+void
+TestI::asyncException(const Current&)
+{
+    //
+    // Only relevant for AMD.
+    //
+}
+
+void
 TestI::shutdown(const Current& current)
 {
     current.adapter->deactivate();
diff --git a/cpp/test/Ice/servantLocator/TestI.h b/cpp/test/Ice/servantLocator/TestI.h
index f912ab5e854..9ee28135673 100644
--- a/cpp/test/Ice/servantLocator/TestI.h
+++ b/cpp/test/Ice/servantLocator/TestI.h
@@ -30,6 +30,9 @@ public:
     virtual ::std::string impossibleException(bool, const Ice::Current&);
     virtual ::std::string intfUserException(bool, const Ice::Current&);
 
+    virtual void asyncResponse(const Ice::Current&);
+    virtual void asyncException(const Ice::Current&);
+
     virtual void shutdown(const Ice::Current&);
 };