An unhandled exception sometimes occured when updating timers. hmails…

…erver#30

hmailserver/source/Server/Common/Application/TimeoutCalculator.cpp

@@ -28,9 +28,9 @@ namespace HM
 
       double newTimeout = maxSecs;
 
-      // If the load is too high, decrease the timeout to 15 seconds.
+      // If the load is too high, decrease the timeout to 60 seconds.
       if (loadPercentage >= 70)
-         newTimeout = 15;
+         newTimeout = 60;
 
       // If we're below the minimal value, increase slightly.
       if (newTimeout < minSecs)

hmailserver/source/Server/Common/Application/TimeoutCalculator.h

@@ -12,7 +12,6 @@ namespace HM
 
       enum Constants
       {
-         // This is the point where performance will decrease (a guessed value).
          MaxConnectionCountOptimized = 20000
       };
 

hmailserver/source/Server/Common/TCPIP/TCPConnection.cpp

@@ -36,7 +36,6 @@ namespace HM
       timer_(io_service),
       receive_binary_(false),
       remote_port_(0),
-      has_timeout_(false),
       receive_buffer_(250000),
       disconnected_(disconnected),
       context_(context),
@@ -58,15 +57,6 @@ namespace HM
 
       if (disconnected_)
          disconnected_->Set();
-
-      try
-      {
-         CancelLogoutTimer();
-      }
-      catch(...)
-      {
-         // should never, and must never throw.
-      }
    }
 
    bool
@@ -425,6 +415,8 @@ namespace HM
    void 
    TCPConnection::AsyncRead(const AnsiString &delimitor)
    {
+      UpdateAutoLogoutTimer();
+
       function<void (const boost::system::error_code&, size_t)> AsyncReadCompletedFunction =
          boost::bind(&TCPConnection::AsyncReadCompleted, shared_from_this(),
          boost::asio::placeholders::error,
@@ -449,12 +441,13 @@ namespace HM
          }
       }
 
-      UpdateLogoutTimer();
    }
 
    void 
    TCPConnection::AsyncReadCompleted(const boost::system::error_code& error,  size_t bytes_transferred)
    {
+      UpdateAutoLogoutTimer();
+
       if (error.value() != 0)
       {
          if (connection_state_ != StateConnected)
@@ -480,12 +473,6 @@ namespace HM
       }
       else
       {
-         // Disable the logout timer while we're parsing data. We don't want to terminate
-         // a client just because he has issued a long-running command. If we do this, we
-         // would have to take care of the fact that we're dropping a connection despite it
-         // still being active.
-         CancelLogoutTimer();
-
          if (receive_binary_)
          {
             shared_ptr<ByteBuffer> pBuffer = shared_ptr<ByteBuffer>(new ByteBuffer());
@@ -584,6 +571,8 @@ namespace HM
    void 
    TCPConnection::AsyncWrite(shared_ptr<ByteBuffer> buffer)
    {
+      UpdateAutoLogoutTimer();
+
       function<void (const boost::system::error_code&, size_t)> AsyncWriteCompletedFunction =
          boost::bind(&TCPConnection::AsyncWriteCompleted, shared_from_this(),
          boost::asio::placeholders::error,
@@ -596,12 +585,14 @@ namespace HM
          boost::asio::async_write
          (socket_, boost::asio::buffer(buffer->GetCharBuffer(), buffer->GetSize()), AsyncWriteCompletedFunction);
 
-      UpdateLogoutTimer();
+
    }
 
    void 
    TCPConnection::AsyncWriteCompleted(const boost::system::error_code& error,  size_t bytes_transferred)
    {
+      UpdateAutoLogoutTimer();
+
       if (error.value() != 0)
       {
          if (connection_state_ != StateConnected)
@@ -715,8 +706,17 @@ namespace HM
 
 
    void 
-   TCPConnection::UpdateLogoutTimer()
+   TCPConnection::UpdateAutoLogoutTimer()
    {
+      /*
+         The timer instance is not thread safe for multiple concurrent callers.
+
+         We may have multiple IO operations starting and stopping at the same time, so we need to synchronize
+         lock to the timer from preventing multiple threads from accessing at the same time.
+      */
+
+      boost::mutex::scoped_lock lock(autologout_timer_);
+
       boost::system::error_code error_code;
 
       // Put a timeout...
@@ -732,19 +732,6 @@ namespace HM
          boost::weak_ptr<TCPConnection>(shared_from_this()), _1));
    }
 
-   void 
-   TCPConnection::CancelLogoutTimer()
-   {
-      boost::system::error_code error_code;
-      timer_.cancel(error_code);
-
-      if (error_code)
-      {
-         ReportError(ErrorManager::Low, 5211, "TCPConnection::CancelLogoutTimer", "Failed to logout timer", error_code);
-      }
-   }
-
-
    void
    TCPConnection::OnTimeout(boost::weak_ptr<TCPConnection> connection, boost::system::error_code const& err)
    {
@@ -770,17 +757,26 @@ namespace HM
    void 
    TCPConnection::Timeout()
    {
-      if (has_timeout_)
+      if (connection_state_ != StateConnected)
       {
-         // We've already posted a timeout once. Disconnect now.
+         // We have already enqueued a disconnect. Since we're still alive, disconnect the socket.
          Disconnect();
          return;
       }
 
-      has_timeout_ = true;
+      // If we will attempt to send more data to the client, such as a timeout message, that message
+      // will have 5 seconds before we give up.
+      SetTimeout(5);
 
+      // Let deriving clients send a disconnect message.
       OnConnectionTimeout();
+
+      // Queue up a disconnection.
       EnqueueDisconnect();
+
+      // Make sure the autologout timer is triggered. This is done in OnConnectionTimeout if we send
+      // a timeout message, but if we don't we need to make sure its triggerd.
+      UpdateAutoLogoutTimer();
    }
 
    void 

hmailserver/source/Server/Common/TCPIP/TCPConnection.h

@@ -51,8 +51,7 @@ namespace HM
       IPAddress GetRemoteEndpointAddress();
       unsigned long GetLocalEndpointPort();
 
-      void UpdateLogoutTimer();
-      void CancelLogoutTimer();
+      void UpdateAutoLogoutTimer();
 
       void SetSecurityRange(shared_ptr<SecurityRange> securityRange);
       shared_ptr<SecurityRange> GetSecurityRange();
@@ -131,7 +130,6 @@ namespace HM
       bool receive_binary_;
       ConnectionSecurity connection_security_;
       long remote_port_;
-      bool has_timeout_;
       String remote_ip_address_;
 
       shared_ptr<SecurityRange> security_range_;
@@ -145,6 +143,7 @@ namespace HM
       bool is_client_;
 
       boost::atomic<ConnectionState> connection_state_;
+      boost::mutex autologout_timer_;
    };
 
 }

hmailserver/source/Server/POP3/POP3Connection.cpp

@@ -335,6 +335,7 @@ namespace HM
       // Fix for mailbox remailing locked even after timeout
       // http://www.hmailserver.com/forum/viewtopic.php?f=7&t=22361
       UnlockMailbox_();
+
       String sMessage = "-ERR Autologout; idle too long\r\n";
       EnqueueWrite(sMessage);
    }
@@ -905,7 +906,8 @@ namespace HM
                messagesToDelete.insert(message->GetUID());
          }
 
-         Application::Instance()->GetFolderManager()->DeleteInboxMessages((int) account_->GetID(), messagesToDelete, boost::bind(&POP3Connection::UpdateLogoutTimer, this));
+         Application::Instance()->GetFolderManager()->DeleteInboxMessages((int) account_->GetID(), messagesToDelete, 
+            boost::bind(&POP3Connection::UpdateAutoLogoutTimer, this));
       }
    }
 

hmailserver/source/Server/SMTP/SMTPConnection.cpp

@@ -1485,16 +1485,14 @@ namespace HM
    void
    SMTPConnection::OnConnectionTimeout()
    {
-      ResetCurrentMessage_();
-
-      EnqueueWrite_("421 Connection timeout.\r\n");
+      EnqueueWrite_("421 Connection timeout.");
    }
 
    void
    SMTPConnection::OnExcessiveDataReceived()
    {
       ResetCurrentMessage_();
-      EnqueueWrite_("421 Excessive amounts of data sent to server.\r\n");
+      EnqueueWrite_("421 Excessive amounts of data sent to server.");
    }
 
    void 
@@ -1583,7 +1581,7 @@ namespace HM
       if (crash_simulation_mode > 0)
          CrashSimulation::Execute(crash_simulation_mode);
 
-      EnqueueWrite_("211 DATA HELO EHLO MAIL NOOP QUIT RCPT RSET SAML TURN VRFY\r\n");
+      EnqueueWrite_("211 DATA HELO EHLO MAIL NOOP QUIT RCPT RSET SAML TURN VRFY");
    }
 
    void