Remove RequestHandlerFactory in C++ (#1789)

cpp/src/Ice/ConnectRequestHandler.cpp

@@ -4,7 +4,6 @@
 
 #include <Ice/ConnectRequestHandler.h>
 #include <Ice/ConnectionRequestHandler.h>
-#include <Ice/RequestHandlerFactory.h>
 #include <Ice/Instance.h>
 #include <Ice/Proxy.h>
 #include <Ice/ConnectionI.h>
@@ -159,20 +158,6 @@ ConnectRequestHandler::setException(exception_ptr ex)
         _exception = ex;
     }
 
-    //
-    // NOTE: remove the request handler *before* notifying the requests that the connection
-    // failed. It's important to ensure that future invocations will obtain a new connect
-    // request handler once invocations are notified.
-    //
-    try
-    {
-        _reference->getInstance()->requestHandlerFactory()->removeRequestHandler(_reference, shared_from_this());
-    }
-    catch(const Ice::CommunicatorDestroyedException&)
-    {
-        // Ignore
-    }
-
     for (deque<ProxyOutgoingAsyncBasePtr>::const_iterator p = _requests.begin(); p != _requests.end(); ++p)
     {
         if ((*p)->exception(ex))
@@ -254,10 +239,6 @@ ConnectRequestHandler::flushRequests()
         catch(const RetryException& ex)
         {
             exception = ex.get();
-
-            // Remove the request handler before retrying.
-            _reference->getInstance()->requestHandlerFactory()->removeRequestHandler(_reference, shared_from_this());
-
             req->retryException();
         }
         catch(const Ice::LocalException&)
@@ -278,13 +259,6 @@ ConnectRequestHandler::flushRequests()
         swap(_exception, exception);
         _initialized = !_exception;
         _flushing = false;
-
-        //
-        // Only remove once all the requests are flushed to
-        // guarantee serialization.
-        //
-        _reference->getInstance()->requestHandlerFactory()->removeRequestHandler(_reference, shared_from_this());
-
         _conditionVariable.notify_all();
     }
 }

cpp/src/Ice/Instance.cpp

@@ -24,7 +24,6 @@
 #include <Ice/EndpointFactoryManager.h>
 #include <Ice/IPEndpointI.h> // For EndpointHostResolver
 #include <Ice/WSEndpoint.h>
-#include <Ice/RequestHandlerFactory.h>
 #include <Ice/RetryQueue.h>
 #include <Ice/DynamicLibrary.h>
 #include <Ice/PluginManagerI.h>
@@ -350,20 +349,6 @@ IceInternal::Instance::referenceFactory() const
     return _referenceFactory;
 }
 
-RequestHandlerFactoryPtr
-IceInternal::Instance::requestHandlerFactory() const
-{
-    lock_guard lock(_mutex);
-
-    if(_state == StateDestroyed)
-    {
-        throw CommunicatorDestroyedException(__FILE__, __LINE__);
-    }
-
-    assert(_requestHandlerFactory);
-    return _requestHandlerFactory;
-}
-
 ProxyFactoryPtr
 IceInternal::Instance::proxyFactory() const
 {
@@ -1227,8 +1212,6 @@ IceInternal::Instance::initialize(const Ice::CommunicatorPtr& communicator)
 
         _referenceFactory = make_shared<ReferenceFactory>(shared_from_this(), communicator);
 
-        _requestHandlerFactory = make_shared<RequestHandlerFactory>(shared_from_this());
-
         _proxyFactory = make_shared<ProxyFactory>(shared_from_this());
 
         const bool isIPv6Supported = IceInternal::isIPv6Supported();
@@ -1713,7 +1696,6 @@ IceInternal::Instance::destroy()
         _timer = nullptr;
 
         _referenceFactory = nullptr;
-        _requestHandlerFactory = nullptr;
         _proxyFactory = nullptr;
         _routerManager = nullptr;
         _locatorManager = nullptr;

cpp/src/Ice/Instance.h

@@ -49,9 +49,6 @@ using TimerPtr = std::shared_ptr<Timer>;
 class MetricsAdminI;
 using MetricsAdminIPtr = std::shared_ptr<MetricsAdminI>;
 
-class RequestHandlerFactory;
-using RequestHandlerFactoryPtr = std::shared_ptr<RequestHandlerFactory>;
-
 class ProxyFactory;
 using ProxyFactoryPtr = std::shared_ptr<ProxyFactory>;
 
@@ -86,7 +83,6 @@ class Instance : public std::enable_shared_from_this<Instance>
     RouterManagerPtr routerManager() const;
     LocatorManagerPtr locatorManager() const;
     ReferenceFactoryPtr referenceFactory() const;
-    RequestHandlerFactoryPtr requestHandlerFactory() const;
     ProxyFactoryPtr proxyFactory() const;
     OutgoingConnectionFactoryPtr outgoingConnectionFactory() const;
     ObjectAdapterFactoryPtr objectAdapterFactory() const;
@@ -171,7 +167,6 @@ class Instance : public std::enable_shared_from_this<Instance>
     RouterManagerPtr _routerManager;
     LocatorManagerPtr _locatorManager;
     ReferenceFactoryPtr _referenceFactory;
-    RequestHandlerFactoryPtr _requestHandlerFactory;
     ProxyFactoryPtr _proxyFactory;
     OutgoingConnectionFactoryPtr _outgoingConnectionFactory;
     ObjectAdapterFactoryPtr _objectAdapterFactory;

cpp/src/Ice/Reference.cpp

@@ -18,8 +18,10 @@
 #include <Ice/LoggerUtil.h>
 #include <Ice/TraceLevels.h>
 #include <Ice/HashUtil.h>
-#include <Ice/RequestHandlerFactory.h>
-#include <Ice/ConnectionRequestHandler.h>
+#include "CollocatedRequestHandler.h"
+#include "ConnectRequestHandler.h"
+#include "ConnectionRequestHandler.h"
+#include "ObjectAdapterFactory.h"
 #include <Ice/DefaultsAndOverrides.h>
 #include <Ice/Comparable.h>
 #include <Ice/StringUtil.h>
@@ -448,6 +450,8 @@ IceInternal::Reference::Reference(const InstancePtr& instance,
                                   int invocationTimeout,
                                   const Ice::Context& ctx) :
     _instance(instance),
+    _overrideCompress(false),
+    _compress(false),
     _communicator(communicator),
     _mode(mode),
     _secure(secure),
@@ -457,15 +461,15 @@ IceInternal::Reference::Reference(const InstancePtr& instance,
     _hashInitialized(false),
     _protocol(protocol),
     _encoding(encoding),
-    _invocationTimeout(invocationTimeout),
-    _overrideCompress(false),
-    _compress(false)
+    _invocationTimeout(invocationTimeout)
 {
 }
 
 IceInternal::Reference::Reference(const Reference& r) :
     enable_shared_from_this<Reference>(),
     _instance(r._instance),
+    _overrideCompress(r._overrideCompress),
+    _compress(r._compress),
     _communicator(r._communicator),
     _mode(r._mode),
     _secure(r._secure),
@@ -475,9 +479,7 @@ IceInternal::Reference::Reference(const Reference& r) :
     _hashInitialized(false),
     _protocol(r._protocol),
     _encoding(r._encoding),
-    _invocationTimeout(r._invocationTimeout),
-    _overrideCompress(r._overrideCompress),
-    _compress(r._compress)
+    _invocationTimeout(r._invocationTimeout)
 {
 }
 
@@ -1401,8 +1403,29 @@ IceInternal::RoutableReference::clone() const
 RequestHandlerPtr
 IceInternal::RoutableReference::getRequestHandler() const
 {
-    return getInstance()->requestHandlerFactory()->getRequestHandler(
-        dynamic_pointer_cast<RoutableReference>(const_cast<RoutableReference*>(this)->shared_from_this()));
+    auto self = const_cast<RoutableReference*>(this)->shared_from_this();
+
+    if (_collocationOptimized)
+    {
+        Ice::ObjectAdapterPtr adapter = _instance->objectAdapterFactory()->findObjectAdapter(self);
+        if(adapter)
+        {
+            return make_shared<CollocatedRequestHandler>(self, adapter);
+        }
+    }
+
+    ConnectRequestHandlerPtr handler = make_shared<ConnectRequestHandler>(self);
+    getConnectionAsync(
+        [handler](Ice::ConnectionIPtr connection, bool compress)
+        {
+            handler->setConnection(connection, compress);
+        },
+        [handler](exception_ptr ex)
+        {
+            handler->setException(ex);
+        });
+
+    return handler;
 }
 
 BatchRequestQueuePtr

cpp/src/Ice/Reference.h

@@ -150,9 +150,12 @@ class Reference : public std::enable_shared_from_this<Reference>
 
     virtual Ice::Int hashInit() const;
 
+    const InstancePtr _instance;
+    bool _overrideCompress;
+    bool _compress; // Only used if _overrideCompress == true
+
 private:
 
-    const InstancePtr _instance;
     const Ice::CommunicatorPtr _communicator;
 
     Mode _mode;
@@ -166,11 +169,6 @@ class Reference : public std::enable_shared_from_this<Reference>
     Ice::ProtocolVersion _protocol;
     Ice::EncodingVersion _encoding;
     int _invocationTimeout;
-
-protected:
-
-    bool _overrideCompress;
-    bool _compress; // Only used if _overrideCompress == true
 };
 
 class FixedReference : public Reference

cpp/src/Ice/msbuild/ice/ice.vcxproj

@@ -567,7 +567,6 @@
     <ClCompile Include="..\..\ReferenceFactory.cpp" />
     <ClCompile Include="..\..\RegisterPluginsInit.cpp" />
     <ClCompile Include="..\..\RequestHandler.cpp" />
-    <ClCompile Include="..\..\RequestHandlerFactory.cpp" />
     <ClCompile Include="..\..\RetryQueue.cpp" />
     <ClCompile Include="..\..\RouterInfo.cpp" />
     <ClCompile Include="..\..\Selector.cpp" />

cpp/src/Ice/msbuild/ice/ice.vcxproj.filters

@@ -450,9 +450,6 @@
     <ClCompile Include="..\..\RequestHandler.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>
-    <ClCompile Include="..\..\RequestHandlerFactory.cpp">
-      <Filter>Source Files</Filter>
-    </ClCompile>
     <ClCompile Include="..\..\RetryQueue.cpp">
       <Filter>Source Files</Filter>
     </ClCompile>

cpp/test/Ice/hold/AllTests.cpp

@@ -184,12 +184,14 @@ allTests(Test::TestHelper* helper)
     {
         int value = 0;
         holdSerialized->set(value, 0);
+        // We use the same proxy for all oneway calls.
+        auto holdSerializedOneway = holdSerialized->ice_oneway();
+
         shared_ptr<promise<void>> completed;
         for(int i = 0; i < 10000; ++i)
         {
             completed = make_shared<promise<void>>();
-            // Create a new proxy for each request
-            holdSerialized->ice_oneway()->setOnewayAsync(value + 1, value,
+            holdSerializedOneway->setOnewayAsync(value + 1, value,
                 nullptr,
                 [](exception_ptr)
                 {