Initial implementation of ConnectTimeout and CloseTimeout in C++ (#2046)

cpp/src/Ice/ConnectionFactory.cpp

@@ -34,7 +34,6 @@ namespace IceInternal
 #endif
 
 using namespace std;
-using namespace std::literals::chrono_literals;
 using namespace Ice;
 using namespace Ice::Instrumentation;
 using namespace IceInternal;
@@ -349,16 +348,10 @@ IceInternal::OutgoingConnectionFactory::OutgoingConnectionFactory(
     : _communicator(communicator),
       _instance(instance),
       _monitor(new FactoryACMMonitor(instance, instance->clientACM())),
-      _idleTimeout(60s),
-      _enableIdleCheck(false),
+      _connectionOptions(instance->clientConnectionOptions()),
       _destroyed(false),
       _pendingConnectCount(0)
 {
-    const PropertiesPtr& properties = _instance->initializationData().properties;
-    int idleTimeout =
-        properties->getPropertyAsIntWithDefault("Ice.IdleTimeout", static_cast<int>(_idleTimeout.count()));
-    _enableIdleCheck = properties->getPropertyAsIntWithDefault("Ice.EnableIdleCheck", _enableIdleCheck ? 1 : 0) > 0;
-    _idleTimeout = chrono::seconds(idleTimeout);
 }
 
 IceInternal::OutgoingConnectionFactory::~OutgoingConnectionFactory()
@@ -595,26 +588,17 @@ IceInternal::OutgoingConnectionFactory::createConnection(const TransceiverPtr& t
             throw Ice::CommunicatorDestroyedException(__FILE__, __LINE__);
         }
 
-        auto idleTimeout = _idleTimeout;
-        bool enableIdleCheck = _enableIdleCheck;
-
-        if (ci.endpoint->datagram())
-        {
-            // No idle timeout or idle check for UDP connections.
-            idleTimeout = chrono::seconds::zero();
-            enableIdleCheck = false;
-        }
+        // The connect, close and idle timeouts are ignored for UDP connections.
 
         connection = ConnectionI::create(
             _communicator,
             _instance,
             _monitor,
             transceiver,
-            idleTimeout,
-            enableIdleCheck,
             ci.connector,
             ci.endpoint->compress(false),
-            nullptr);
+            nullptr,
+            _connectionOptions);
     }
     catch (const Ice::LocalException&)
     {
@@ -1510,11 +1494,10 @@ IceInternal::IncomingConnectionFactory::message(ThreadPoolCurrent& current)
                 _instance,
                 _monitor,
                 transceiver,
-                _idleTimeout,
-                _enableIdleCheck,
                 nullptr, // connector
                 _endpoint,
-                _adapter);
+                _adapter,
+                _connectionOptions);
         }
         catch (const LocalException& ex)
         {
@@ -1666,8 +1649,7 @@ IceInternal::IncomingConnectionFactory::IncomingConnectionFactory(
     const shared_ptr<ObjectAdapterI>& adapter)
     : _instance(instance),
       _monitor(new FactoryACMMonitor(instance, dynamic_cast<ObjectAdapterI*>(adapter.get())->getACM())),
-      _idleTimeout(60s),
-      _enableIdleCheck(false),
+      _connectionOptions(instance->serverConnectionOptions(adapter->getName())),
       _endpoint(endpoint),
       _publishedEndpoint(publishedEndpoint),
       _acceptorStarted(false),
@@ -1727,8 +1709,6 @@ IceInternal::IncomingConnectionFactory::initialize()
         if (_transceiver)
         {
             // All this is for UDP "connections".
-            _idleTimeout = chrono::seconds::zero(); // always disable and ignore properties
-            _enableIdleCheck = false;
 
             if (_instance->traceLevels()->network >= 2)
             {
@@ -1741,33 +1721,15 @@ IceInternal::IncomingConnectionFactory::initialize()
                 _instance,
                 nullptr,
                 _transceiver,
-                _idleTimeout,
-                _enableIdleCheck,
                 nullptr,
                 _endpoint,
-                _adapter));
+                _adapter,
+                _connectionOptions));
             connection->startAsync(nullptr, nullptr);
             _connections.insert(connection);
         }
         else
         {
-            const PropertiesPtr& properties = _instance->initializationData().properties;
-            int idleTimeout =
-                properties->getPropertyAsIntWithDefault("Ice.IdleTimeout", static_cast<int>(_idleTimeout.count()));
-            _enableIdleCheck =
-                properties->getPropertyAsIntWithDefault("Ice.EnableIdleCheck", _enableIdleCheck ? 0 : 1) > 0;
-
-            string adapterName = _adapter->getName();
-            if (!adapterName.empty())
-            {
-                // Override if any of these properties are set, otherwise keep previous value.
-                idleTimeout = properties->getPropertyAsIntWithDefault(adapterName + ".IdleTimeout", idleTimeout);
-                _enableIdleCheck = properties->getPropertyAsIntWithDefault(
-                                       adapterName + ".EnableIdleCheck",
-                                       _enableIdleCheck ? 0 : 1) > 0;
-            }
-            _idleTimeout = chrono::seconds(idleTimeout);
-
 #if TARGET_OS_IPHONE != 0
             //
             // The notification center will call back on the factory to

cpp/src/Ice/ConnectionFactory.h

@@ -149,8 +149,8 @@ namespace IceInternal
         Ice::CommunicatorPtr _communicator;
         const InstancePtr _instance;
         const FactoryACMMonitorPtr _monitor;
-        std::chrono::seconds _idleTimeout;
-        bool _enableIdleCheck;
+        const Ice::ConnectionOptions _connectionOptions;
+
         bool _destroyed;
 
         using ConnectCallbackSet = std::set<ConnectCallbackPtr>;
@@ -235,8 +235,7 @@ namespace IceInternal
 
         const InstancePtr _instance;
         const FactoryACMMonitorPtr _monitor;
-        std::chrono::seconds _idleTimeout;
-        bool _enableIdleCheck;
+        const Ice::ConnectionOptions _connectionOptions;
 
         AcceptorPtr _acceptor;
         const TransceiverPtr _transceiver;

cpp/src/Ice/ConnectionI.cpp

@@ -2097,7 +2097,8 @@ Ice::ConnectionI::ConnectionI(
     const TransceiverPtr& transceiver,
     const ConnectorPtr& connector,
     const EndpointIPtr& endpoint,
-    const shared_ptr<ObjectAdapterI>& adapter) noexcept
+    const shared_ptr<ObjectAdapterI>& adapter,
+    const ConnectionOptions& options) noexcept
     : _communicator(communicator),
       _instance(instance),
       _monitor(monitor),
@@ -2115,6 +2116,9 @@ Ice::ConnectionI::ConnectionI(
       _writeTimeoutScheduled(false),
       _readTimeout(new TimeoutCallback(this)),
       _readTimeoutScheduled(false),
+      _connectTimeout(options.connectTimeout),
+      _closeTimeout(options.closeTimeout),
+      _inactivityTimeout(options.inactivityTimeout),
       _warn(_instance->initializationData().properties->getPropertyAsInt("Ice.Warn.Connections") > 0),
       _warnUdp(_instance->initializationData().properties->getPropertyAsInt("Ice.Warn.Datagrams") > 0),
       _compressionLevel(1),
@@ -2156,17 +2160,19 @@ Ice::ConnectionI::create(
     const InstancePtr& instance,
     const ACMMonitorPtr& monitor,
     const TransceiverPtr& transceiver,
-    const chrono::seconds& idleTimeout,
-    bool enableIdleCheck,
     const ConnectorPtr& connector,
     const EndpointIPtr& endpoint,
-    const shared_ptr<ObjectAdapterI>& adapter)
+    const shared_ptr<ObjectAdapterI>& adapter,
+    const ConnectionOptions& options)
 {
     shared_ptr<IdleTimeoutTransceiverDecorator> decoratedTransceiver;
-    if (idleTimeout > chrono::milliseconds::zero())
+    if (options.idleTimeout > chrono::milliseconds::zero() && !endpoint->datagram())
     {
-        decoratedTransceiver =
-            make_shared<IdleTimeoutTransceiverDecorator>(transceiver, idleTimeout, enableIdleCheck, instance->timer());
+        decoratedTransceiver = make_shared<IdleTimeoutTransceiverDecorator>(
+            transceiver,
+            options.idleTimeout,
+            options.enableIdleCheck,
+            instance->timer());
     }
 
     Ice::ConnectionIPtr connection(new ConnectionI(
@@ -2176,7 +2182,8 @@ Ice::ConnectionI::create(
         decoratedTransceiver ? decoratedTransceiver : transceiver,
         connector,
         endpoint,
-        adapter));
+        adapter,
+        options));
 
     if (decoratedTransceiver)
     {
@@ -3535,15 +3542,7 @@ Ice::ConnectionI::scheduleTimeout(SocketOperation status)
     int timeout;
     if (_state < StateActive)
     {
-        DefaultsAndOverridesPtr defaultsAndOverrides = _instance->defaultsAndOverrides();
-        if (defaultsAndOverrides->overrideConnectTimeout)
-        {
-            timeout = defaultsAndOverrides->overrideConnectTimeoutValue;
-        }
-        else
-        {
-            timeout = _endpoint->timeout();
-        }
+        timeout = static_cast<int>(chrono::milliseconds(_connectTimeout).count());
     }
     else if (_state < StateClosingPending)
     {
@@ -3555,15 +3554,7 @@ Ice::ConnectionI::scheduleTimeout(SocketOperation status)
     }
     else
     {
-        DefaultsAndOverridesPtr defaultsAndOverrides = _instance->defaultsAndOverrides();
-        if (defaultsAndOverrides->overrideCloseTimeout)
-        {
-            timeout = defaultsAndOverrides->overrideCloseTimeoutValue;
-        }
-        else
-        {
-            timeout = _endpoint->timeout();
-        }
+        timeout = static_cast<int>(chrono::milliseconds(_closeTimeout).count());
     }
 
     if (timeout < 0)

cpp/src/Ice/ConnectionI.h

@@ -7,6 +7,7 @@
 
 #include "ACM.h"
 #include "ConnectionFactoryF.h"
+#include "ConnectionOptions.h"
 #include "ConnectorF.h"
 #include "EndpointIF.h"
 #include "EventHandler.h"
@@ -238,18 +239,18 @@ namespace Ice
             const IceInternal::TransceiverPtr&,
             const IceInternal::ConnectorPtr&,
             const IceInternal::EndpointIPtr&,
-            const std::shared_ptr<ObjectAdapterI>&) noexcept;
+            const std::shared_ptr<ObjectAdapterI>&,
+            const ConnectionOptions&) noexcept;
 
         static ConnectionIPtr create(
             const Ice::CommunicatorPtr&,
             const IceInternal::InstancePtr&,
             const IceInternal::ACMMonitorPtr&,
             const IceInternal::TransceiverPtr&,
-            const std::chrono::seconds& idleTimeout,
-            bool enableIdleCheck,
             const IceInternal::ConnectorPtr&,
             const IceInternal::EndpointIPtr&,
-            const std::shared_ptr<ObjectAdapterI>&);
+            const std::shared_ptr<ObjectAdapterI>&,
+            const ConnectionOptions&);
 
         enum State
         {
@@ -334,6 +335,10 @@ namespace Ice
         const IceUtil::TimerTaskPtr _readTimeout;
         bool _readTimeoutScheduled;
 
+        const std::chrono::seconds _connectTimeout;
+        const std::chrono::seconds _closeTimeout;
+        const std::chrono::seconds _inactivityTimeout;
+
         std::function<void(ConnectionIPtr)> _connectionStartCompleted;
         std::function<void(ConnectionIPtr, std::exception_ptr)> _connectionStartFailed;
 

cpp/src/Ice/ConnectionOptions.h

@@ -0,0 +1,23 @@
+//
+// Copyright (c) ZeroC, Inc. All rights reserved.
+//
+
+#ifndef ICE_CONNECTION_OPTIONS_H
+#define ICE_CONNECTION_OPTIONS_H
+
+#include <chrono>
+
+namespace Ice
+{
+    /// Represents a property bag used to configure client and server connections.
+    struct ConnectionOptions
+    {
+        std::chrono::seconds connectTimeout = std::chrono::seconds(10);
+        std::chrono::seconds closeTimeout = std::chrono::seconds(10);
+        std::chrono::seconds idleTimeout = std::chrono::seconds(60);
+        bool enableIdleCheck = false; // TODO: switch to true
+        std::chrono::seconds inactivityTimeout = std::chrono::seconds(300);
+    };
+}
+
+#endif

cpp/src/Ice/DefaultsAndOverrides.cpp

@@ -14,10 +14,6 @@ using namespace IceInternal;
 IceInternal::DefaultsAndOverrides::DefaultsAndOverrides(const PropertiesPtr& properties, const LoggerPtr& logger)
     : overrideTimeout(false),
       overrideTimeoutValue(-1),
-      overrideConnectTimeout(false),
-      overrideConnectTimeoutValue(-1),
-      overrideCloseTimeout(false),
-      overrideCloseTimeoutValue(-1),
       overrideCompress(false),
       overrideCompressValue(false),
       overrideSecure(false),
@@ -56,34 +52,6 @@ IceInternal::DefaultsAndOverrides::DefaultsAndOverrides(const PropertiesPtr& pro
         }
     }
 
-    value = properties->getProperty("Ice.Override.ConnectTimeout");
-    if (!value.empty())
-    {
-        const_cast<bool&>(overrideConnectTimeout) = true;
-        const_cast<int32_t&>(overrideConnectTimeoutValue) = properties->getPropertyAsInt("Ice.Override.ConnectTimeout");
-        if (overrideConnectTimeoutValue < 1 && overrideConnectTimeoutValue != -1)
-        {
-            const_cast<int32_t&>(overrideConnectTimeoutValue) = -1;
-            Warning out(logger);
-            out << "invalid value for Ice.Override.ConnectTimeout `"
-                << properties->getProperty("Ice.Override.ConnectTimeout") << "': defaulting to -1";
-        }
-    }
-
-    value = properties->getProperty("Ice.Override.CloseTimeout");
-    if (!value.empty())
-    {
-        const_cast<bool&>(overrideCloseTimeout) = true;
-        const_cast<int32_t&>(overrideCloseTimeoutValue) = properties->getPropertyAsInt("Ice.Override.CloseTimeout");
-        if (overrideCloseTimeoutValue < 1 && overrideCloseTimeoutValue != -1)
-        {
-            const_cast<int32_t&>(overrideCloseTimeoutValue) = -1;
-            Warning out(logger);
-            out << "invalid value for Ice.Override.CloseTimeout `"
-                << properties->getProperty("Ice.Override.CloseTimeout") << "': defaulting to -1";
-        }
-    }
-
     value = properties->getProperty("Ice.Override.Compress");
     if (!value.empty())
     {

cpp/src/Ice/DefaultsAndOverrides.h

@@ -34,10 +34,6 @@ namespace IceInternal
 
         bool overrideTimeout;
         std::int32_t overrideTimeoutValue;
-        bool overrideConnectTimeout;
-        std::int32_t overrideConnectTimeoutValue;
-        bool overrideCloseTimeout;
-        std::int32_t overrideCloseTimeoutValue;
         bool overrideCompress;
         bool overrideCompressValue;
         bool overrideSecure;