rewrote connection validation code

6 files changed, 113 insertions, 166 deletions

diff --git a/cpp/src/Ice/Connection.cpp b/cpp/src/Ice/Connection.cpp
index a48b9649f4a..0a824b0a0fa 100644
--- a/cpp/src/Ice/Connection.cpp
+++ b/cpp/src/Ice/Connection.cpp
@@ -39,6 +39,85 @@ IceInternal::Connection::destroyed() const
 }
 
 void
+IceInternal::Connection::validate()
+{
+    IceUtil::RecMutex::Lock sync(*this);
+
+    if(_endpoint->datagram())
+    {
+	//
+	// Datagram connections are always implicitly validated.
+	//
+	return;
+    }
+
+    try
+    {
+	if(_adapter)
+	{
+	    //
+	    // Incoming connections play the active role with respect to
+	    // connection validation.
+	    //
+	    BasicStream os(_instance);
+	    os.write(protocolVersion);
+	    os.write(encodingVersion);
+	    os.write(validateConnectionMsg);
+	    os.write(headerSize); // Message size.
+	    os.i = os.b.begin();
+	    traceHeader("sending validate connection", os, _logger, _traceLevels);
+	    _transceiver->write(os, _endpoint->timeout());
+	}
+	else
+	{
+	    //
+	    // Outgoing connection play the passive role with respect to
+	    // connection validation.
+	    //
+	    BasicStream is(_instance);
+	    is.b.resize(headerSize);
+	    is.i = is.b.begin();
+	    _transceiver->read(is, _endpoint->timeout());
+	    assert(is.i == is.b.end());
+	    int pos = is.i - is.b.begin();
+	    assert(pos >= headerSize);
+	    is.i = is.b.begin();
+	    Byte protVer;
+	    is.read(protVer);
+	    if(protVer != protocolVersion)
+	    {
+		throw UnsupportedProtocolException(__FILE__, __LINE__);
+	    }
+	    Byte encVer;
+	    is.read(encVer);
+	    if(encVer != encodingVersion)
+	    {
+		throw UnsupportedEncodingException(__FILE__, __LINE__);
+	    }
+	    Byte messageType;
+	    is.read(messageType);
+	    if(messageType != validateConnectionMsg)
+	    {
+		throw ConnectionNotValidatedException(__FILE__, __LINE__);
+	    }
+	    Int size;
+	    is.read(size);
+	    if(size != headerSize)
+	    {
+		throw IllegalMessageSizeException(__FILE__, __LINE__);
+	    }
+	    traceHeader("received validate connection", is, _logger, _traceLevels);
+	}
+    }
+    catch(const LocalException& ex)
+    {
+	setState(StateClosed, ex);
+	assert(_exception.get());
+	_exception->ice_throw();
+    }
+}
+
+void
 IceInternal::Connection::hold()
 {
     IceUtil::RecMutex::Lock sync(*this);
@@ -416,22 +495,6 @@ IceInternal::Connection::message(BasicStream& stream, const ThreadPoolPtr& threa
 	    stream.read(messageType);
 
 	    //
-	    // Check whether the connection is validated.
-	    //
-	    if(!_connectionValidated && messageType != validateConnectionMsg)
-	    {
-		//
-                // Yes, we must set _connectionValidated to true
-                // here. The connection gets implicitly validated by
-                // any kind of message. However, it's still a protocol
-                // error like any other if no explicit connection
-                // validation message was sent first.
-		//
-		_connectionValidated = true;
-		throw ConnectionNotValidatedException(__FILE__, __LINE__);
-	    }
-	    
-	    //
 	    // Uncompress if necessary.
 	    //
 	    if(messageType == compressedRequestMsg ||
@@ -567,18 +630,11 @@ IceInternal::Connection::message(BasicStream& stream, const ThreadPoolPtr& threa
 		case validateConnectionMsg:
 		{
 		    traceHeader("received validate connection", stream, _logger, _traceLevels);
-		    if(_endpoint->datagram())
-		    {
-			if(_warn)
-			{
-			    Warning out(_logger);
-			    out << "ignoring validate connection message for datagram connection:\n"
-				<< _transceiver->toString();
-			}
-		    }
-		    else
+		    if(_warn)
 		    {
-			_connectionValidated = true;
+			Warning out(_logger);
+			out << "ignoring unexpected validate connection message:\n"
+			    << _transceiver->toString();
 		    }
 		    break;
 		}
@@ -822,49 +878,6 @@ IceInternal::Connection::Connection(const InstancePtr& instance,
     _state(StateHolding),
     _registeredWithPool(false)
 {
-    if(_endpoint->datagram())
-    {
-	//
-	// Datagram connections are always implicitly validated.
-	//
-	_connectionValidated = true;
-    }
-    else
-    {
-	if(_adapter)
-	{
-	    //
-	    // Incoming connections play the active role with respect
-	    // to connection validation, and are implicitly validated.
-	    //
-	    try
-	    {
-		validateConnection();
-	    }
-	    catch(const LocalException& ex)
-	    {
-		if(_warn)
-		{
-		    Warning out(_logger);
-		    out << "connection exception:\n" << ex << '\n' << _transceiver->toString();
-		}
-		_transceiver->close();
-		_state = StateClosed;
-		ex.ice_throw();
-	    }
-
-	    _connectionValidated = true;
-	}
-	else
-	{
-	    //
-	    // Outgoing connections are passive with respect to
-	    // validation, i.e., they wait until they get a validate
-	    // connection message from the server.
-	    //
-	    _connectionValidated = false;
-	}
-    }
 }
 
 IceInternal::Connection::~Connection()
@@ -903,28 +916,7 @@ IceInternal::Connection::setState(State state, const LocalException& ex)
 
     if(!_exception.get())
     {
-	if(!_connectionValidated && dynamic_cast<const ConnectionLostException*>(&ex))
-	{
-	    //
-	    // If the connection has not been validated yet, we treat
-	    // a connection loss just as if we would have received a
-	    // close connection messsage. This way, Ice will retry a
-	    // request if the peer just accepts and closes a
-	    // connection. This can happen, for example, if a
-	    // connection is in the server's backlog, but not yet
-	    // accepted by the server. In such case, the connection
-	    // has been established from the client point of view, but
-	    // not yet from the server point of view. If the server
-	    // then closes the acceptor socket, the client will get a
-	    // connection loss without receiving an explicit close
-	    // connection message first.
-	    //
-	    _exception = auto_ptr<LocalException>(new CloseConnectionException(__FILE__, __LINE__));
-	}
-	else
-	{
-	    _exception = auto_ptr<LocalException>(dynamic_cast<LocalException*>(ex.ice_clone()));
-	}
+	_exception = auto_ptr<LocalException>(dynamic_cast<LocalException*>(ex.ice_clone()));
 
 	if(_warn)
 	{
@@ -1039,19 +1031,6 @@ IceInternal::Connection::setState(State state)
 }
 
 void
-IceInternal::Connection::validateConnection() const
-{
-    BasicStream os(_instance);
-    os.write(protocolVersion);
-    os.write(encodingVersion);
-    os.write(validateConnectionMsg);
-    os.write(headerSize); // Message size.
-    os.i = os.b.begin();
-    traceHeader("sending validate connection", os, _logger, _traceLevels);
-    _transceiver->write(os, _endpoint->timeout());
-}
-
-void
 IceInternal::Connection::closeConnection() const
 {
     BasicStream os(_instance);
diff --git a/cpp/src/Ice/Connection.h b/cpp/src/Ice/Connection.h
index f3eaa396925..cf110ddfce8 100644
--- a/cpp/src/Ice/Connection.h
+++ b/cpp/src/Ice/Connection.h
@@ -42,6 +42,7 @@ class Connection : public EventHandler, public ::IceUtil::RecMutex
 public:
 
     bool destroyed() const;
+    void validate();
     void hold();
     void activate();
     void incProxyUsageCount();
@@ -91,7 +92,6 @@ private:
 
     void setState(State, const ::Ice::LocalException&);
     void setState(State);
-    void validateConnection() const;
     void closeConnection() const;
     void registerWithPool();
     void unregisterWithPool();
@@ -116,7 +116,6 @@ private:
     int _proxyUsageCount;
     State _state;
     bool _registeredWithPool;
-    bool _connectionValidated;
 };
 
 }
diff --git a/cpp/src/Ice/ConnectionFactory.cpp b/cpp/src/Ice/ConnectionFactory.cpp
index 7a93c367f59..56bb415d6eb 100644
--- a/cpp/src/Ice/ConnectionFactory.cpp
+++ b/cpp/src/Ice/ConnectionFactory.cpp
@@ -112,19 +112,12 @@ IceInternal::OutgoingConnectionFactory::create(const vector<EndpointPtr>& endpoi
 		assert(transceiver);
 	    }	    
 	    connection = new Connection(_instance, transceiver, endpoint, 0);
+	    connection->validate();
 	    connection->activate();
 	    _connections.insert(make_pair(endpoint, connection));
 	    break;
 	}
-	catch(const SocketException& ex)
-	{
-	    exception = auto_ptr<LocalException>(dynamic_cast<LocalException*>(ex.ice_clone()));
-	}
-	catch(const DNSException& ex)
-	{
-	    exception = auto_ptr<LocalException>(dynamic_cast<LocalException*>(ex.ice_clone()));
-	}
-	catch(const TimeoutException& ex)
+	catch(const LocalException& ex)
 	{
 	    exception = auto_ptr<LocalException>(dynamic_cast<LocalException*>(ex.ice_clone()));
 	}
@@ -366,6 +359,7 @@ IceInternal::IncomingConnectionFactory::message(BasicStream&, const ThreadPoolPt
     {
 	assert(transceiver);
 	ConnectionPtr connection = new Connection(_instance, transceiver, _endpoint, _adapter);
+	connection->validate();
 	connection->activate();
 	_connections.push_back(connection);
     }
@@ -446,6 +440,7 @@ IceInternal::IncomingConnectionFactory::IncomingConnectionFactory(const Instance
 	if(_transceiver)
 	{
 	    ConnectionPtr connection = new Connection(_instance, _transceiver, _endpoint, _adapter);
+	    connection->validate();
 	    _connections.push_back(connection);
 
 	    //
diff --git a/cpp/src/Ice/Network.cpp b/cpp/src/Ice/Network.cpp
index aaf88639909..e408a43cf97 100644
--- a/cpp/src/Ice/Network.cpp
+++ b/cpp/src/Ice/Network.cpp
@@ -27,7 +27,7 @@ IceInternal::interrupted()
     }
 #else
     if(errno == EINTR ||
-	errno == EPROTO)
+       errno == EPROTO)
     {
 	return true;
     }
@@ -49,15 +49,15 @@ IceInternal::acceptInterrupted()
 #ifdef _WIN32
     int error = WSAGetLastError();
     if(error == WSAECONNABORTED ||
-	error == WSAECONNRESET ||
-	error == WSAETIMEDOUT)
+       error == WSAECONNRESET ||
+       error == WSAETIMEDOUT)
     {
 	return true;
     }
 #else
     if(errno == ECONNABORTED ||
-	errno == ECONNRESET ||
-	errno == ETIMEDOUT)
+       errno == ECONNRESET ||
+       errno == ETIMEDOUT)
     {
 	return true;
     }
@@ -74,7 +74,7 @@ IceInternal::noBuffers()
 #ifdef _WIN32
     int error = WSAGetLastError();
     if(error == WSAENOBUFS ||
-	error == WSAEFAULT)
+       error == WSAEFAULT)
     {
 	return true;
     }
@@ -101,7 +101,7 @@ IceInternal::wouldBlock()
     }
 #else
     if(errno == EAGAIN ||
-	errno == EWOULDBLOCK)
+       errno == EWOULDBLOCK)
     {
 	return true;
     }
@@ -118,21 +118,21 @@ IceInternal::connectFailed()
 #ifdef _WIN32
     int error = WSAGetLastError();
     if(error == WSAECONNREFUSED ||
-	error == WSAETIMEDOUT ||
-	error == WSAENETUNREACH ||
-	error == WSAECONNRESET ||
-	error == WSAESHUTDOWN ||
-	error == WSAECONNABORTED)
+       error == WSAETIMEDOUT ||
+       error == WSAENETUNREACH ||
+       error == WSAECONNRESET ||
+       error == WSAESHUTDOWN ||
+       error == WSAECONNABORTED)
     {
 	return true;
     }
 #else
     if(errno == ECONNREFUSED ||
-	errno == ETIMEDOUT ||
-	errno == ENETUNREACH ||
-	errno == ECONNRESET ||
-	errno == ESHUTDOWN ||
-	errno == ECONNABORTED)
+       errno == ETIMEDOUT ||
+       errno == ENETUNREACH ||
+       errno == ECONNRESET ||
+       errno == ESHUTDOWN ||
+       errno == ECONNABORTED)
     {
 	return true;
     }
@@ -170,15 +170,15 @@ IceInternal::connectionLost()
 #ifdef _WIN32
     int error = WSAGetLastError();
     if(error == WSAECONNRESET ||
-	error == WSAESHUTDOWN ||
-	error == WSAECONNABORTED)
+       error == WSAESHUTDOWN ||
+       error == WSAECONNABORTED)
     {
 	return true;
     }
 #else
     if(errno == ECONNRESET ||
-	errno == ESHUTDOWN ||
-	errno == ECONNABORTED)
+       errno == ESHUTDOWN ||
+       errno == ECONNABORTED)
     {
 	return true;
     }
diff --git a/cpp/src/Ice/Proxy.cpp b/cpp/src/Ice/Proxy.cpp
index bb97f9cc691..1875bf99847 100644
--- a/cpp/src/Ice/Proxy.cpp
+++ b/cpp/src/Ice/Proxy.cpp
@@ -574,26 +574,7 @@ IceProxy::Ice::Object::__handleException(const LocalException& ex, int& cnt)
 	_delegate = 0;
     }
 
-    try
-    {
-	ex.ice_throw();
-    }
-    catch(const CloseConnectionException&)
-    {
-	++cnt;
-    }
-    catch(const SocketException&)
-    {
-	++cnt;
-    }
-    catch(const DNSException&)
-    {
-	++cnt;
-    }
-    catch(const TimeoutException&)
-    {
-	++cnt;
-    }
+    ++cnt;
     
     TraceLevelsPtr traceLevels = _reference->instance->traceLevels();
     LoggerPtr logger = _reference->instance->logger();
diff --git a/cpp/src/IcePatch/Server.cpp b/cpp/src/IcePatch/Server.cpp
index 6bc7de74198..ad708191b18 100644
--- a/cpp/src/IcePatch/Server.cpp
+++ b/cpp/src/IcePatch/Server.cpp
@@ -198,20 +198,13 @@ IcePatch::Updater::run()
 	    // Just loop if we're busy.
 	    //
 	}
-	catch(const ConnectFailedException&)
+	catch(const Exception&)
 	{
 	    //
-	    // This exception can be raised if the adapter is shutdown
-	    // while this thread is still running. In such case, we
-	    // terminate this thread.
+	    // Bail out on any other exception.
 	    //
 	    break;
 	}
-	catch(const Exception& ex)
-	{
-	    Error out(_logger);
-	    out << "exception during update:\n" << ex;
-	}
 
 	if(_destroy)
 	{