Added is_stopped flag to server to prevent new connections and removed REUSE socket option

Author: FlorianReimold

fineftp-server/src/server_impl.cpp

@@ -23,6 +23,7 @@ namespace fineftp
   FtpServerImpl::FtpServerImpl(const std::string& address, uint16_t port)
     : port_                 (port)
     , address_              (address)
+    , is_stopped_           (false)
     , acceptor_             (io_service_)
   {}
 
@@ -63,18 +64,19 @@ namespace fineftp
         return false;
       }
     }
-    
-    {
-      const std::lock_guard<std::mutex> acceptor_lock(acceptor_mutex_);
 
-      asio::error_code ec;
-      acceptor_.set_option(asio::ip::tcp::acceptor::reuse_address(true), ec);
-      if (ec)
-      {
-        std::cerr << "Error setting reuse_address option: " << ec.message() << std::endl;
-        return false;
-      }
-    }
+    // TODO: Add the code again to use reuse_address option
+    //{
+    //  const std::lock_guard<std::mutex> acceptor_lock(acceptor_mutex_);
+
+    //  asio::error_code ec;
+    //  acceptor_.set_option(asio::ip::tcp::acceptor::reuse_address(true), ec);
+    //  if (ec)
+    //  {
+    //    std::cerr << "Error setting reuse_address option: " << ec.message() << std::endl;
+    //    return false;
+    //  }
+    //}
 
     {
       const std::lock_guard<std::mutex> acceptor_lock(acceptor_mutex_);
@@ -119,6 +121,8 @@ namespace fineftp
 
   void FtpServerImpl::stop()
   {
+    is_stopped_ = true;
+
     // Prevent new sessions from being created
     {
       const std::lock_guard<std::mutex> acceptor_lock(acceptor_mutex_);
@@ -154,6 +158,11 @@ namespace fineftp
 
   void FtpServerImpl::waitForNextFtpSession()
   {
+    if (is_stopped_)
+    {
+      return;
+    }
+
     auto shutdown_callback = [weak_me = std::weak_ptr<FtpServerImpl>(shared_from_this())](FtpSession* session_to_delete)
                               {
                                 if (auto me = weak_me.lock())
@@ -173,6 +182,16 @@ namespace fineftp
       acceptor_.async_accept(new_ftp_session->getSocket()
                             , [weak_me = std::weak_ptr<FtpServerImpl>(shared_from_this()), new_ftp_session](asio::error_code ec)
                               {
+                                // Lock the shared pointer to this. May be a nullptr, so we need to check!!!
+                                auto me = weak_me.lock();
+
+                                // Before even checking the error code, check if the server has been stopped
+                                if (!me || (me->is_stopped_))
+                                {
+                                  return;
+                                }
+
+                                // Check error code
                                 if (ec)
                                 {
                                   if (ec != asio::error::operation_aborted)
@@ -187,8 +206,6 @@ namespace fineftp
 #endif
                                 // TODO: review if this is thread safe, if right here the ftp server is shut down and the acceptor is closed. I think, that then the session will still be added to the list of open sessions and kept open.
 
-                                auto me = weak_me.lock();
-
                                 if (me)
                                 {
                                   const std::lock_guard<std::mutex> session_list_lock(me->session_list_mutex_);

fineftp-server/src/server_impl.h

@@ -54,10 +54,11 @@ namespace fineftp
     void waitForNextFtpSession();
 
   private:
-    UserDatabase   ftp_users_;
+    UserDatabase      ftp_users_;
 
-    const uint16_t port_;
-    const std::string address_;
+    const uint16_t    port_;                                                    //! Port used on creation. May be 0. In that case, the OS will choose a port. Thus, this variable should not be used for getting the actual port.
+    const std::string address_;                                                 //! Address used on creation. The acceptor is bound to that address.
+    std::atomic<bool> is_stopped_;                                              //! Tells whether the server has been stopped. When stopped, it will refuse to accept new connections.
 
     std::vector<std::thread> thread_pool_;
     asio::io_service         io_service_;

tests/fineftp_test/src/startstop_test.cpp

@@ -214,6 +214,44 @@ TEST(StartStopTests, connection_count)
 }
 #endif
 
+#if 1
+// Restart the server multiple times and check if the same socket can be reused
+TEST(StartStopTests, restart_server)
+{
+  constexpr size_t num_restarts = 10;
+
+  const DirPreparer dir_preparer(1, 1);
+  dir_preparer.create_client_files(std::filesystem::path("test.txt"), 16);
+
+
+  std::unique_ptr<fineftp::FtpServer> server;
+
+  for (unsigned int i = 0; i < num_restarts; ++i)
+  {
+    if (server)
+    {
+      server->stop();
+      server.reset();
+    }
+
+    server = std::make_unique<fineftp::FtpServer>(2121);
+    server->addUserAnonymous(dir_preparer.server_local_root_dir(0).string(), fineftp::Permission::All);
+    server->start(4);
+
+    // use CURL to upload the file to the server
+    const std::string curl_command = "curl -S -s -T " + dir_preparer.client_local_root_dir(0, 0).string() + "/test.txt ftp://localhost:2121/test" + std::to_string(i) + ".txt --user anonymous:anonymous";
+    const int curl_return_code = system_execute(curl_command);
+
+    // Check return code
+    EXPECT_EQ(curl_return_code, 0);
+
+    // Check existence of file
+    EXPECT_TRUE(std::filesystem::exists(dir_preparer.server_local_root_dir(0) / ("test" + std::to_string(i) + ".txt")));
+  }
+}
+
+#endif
+
 #if 1
 // Create a large amount of servers, upload files to them from threads and stop the servers while the upload may still be in progress
 TEST(StartStopTests, multiple_servers_upload_stop)