From 6c2d1bb236c281eb195fef11b51fccec448050bc Mon Sep 17 00:00:00 2001
From: Roel Standaert <roel@emweb.be>
Date: Wed, 8 Mar 2023 16:52:56 +0100
Subject: [PATCH] WT-11302: extend lifetime of closed listeners

When we call handleResume() or handleStop() all acceptors are closed
and all listeners deleted. However, closing the acceptor causes
handleTcpAccept or handleSslAccept to be called with an error code.
Previously, this would mean that the listener argument to these
functions would be a dangling pointer and dereferencing it is undefined
behavior. By letting the callback take a shared_ptr we can extend
the lifetime just a bit longer, so we can safely use it in the callback.
---
 src/http/Server.C | 44 +++++++++++++++++++++++---------------------
 src/http/Server.h | 12 ++++++++----
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/src/http/Server.C b/src/http/Server.C
index b26a44477..e327b703a 100644
--- a/src/http/Server.C
+++ b/src/http/Server.C
@@ -393,8 +393,8 @@ void Server::addTcpEndpoint(const asio::ip::tcp::endpoint &endpoint,
                             const std::string &address,
                             Wt::AsioWrapper::error_code &errc)
 {
-  tcp_listeners_.push_back(TcpListener(asio::ip::tcp::acceptor(wt_.ioService()), TcpConnectionPtr()));
-  asio::ip::tcp::acceptor &tcp_acceptor = tcp_listeners_.back().acceptor;
+  tcp_listeners_.push_back(std::make_shared<TcpListener>(asio::ip::tcp::acceptor(wt_.ioService()), TcpConnectionPtr()));
+  asio::ip::tcp::acceptor &tcp_acceptor = tcp_listeners_.back()->acceptor;
   tcp_acceptor.open(endpoint.protocol());
   tcp_acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true));
 #ifndef WT_WIN32
@@ -406,7 +406,7 @@ void Server::addTcpEndpoint(const asio::ip::tcp::endpoint &endpoint,
 
     LOG_INFO_S(&wt_, "started server: " << addressString("http", endpoint, address));
 
-    tcp_listeners_.back().new_connection.reset
+    tcp_listeners_.back()->new_connection.reset
       (new TcpConnection(wt_.ioService(), this, connection_manager_,
                          request_handler_));
   } else {
@@ -450,8 +450,8 @@ void Server::addSslEndpoint(const asio::ip::tcp::endpoint &endpoint,
                             const std::string &address,
                             Wt::AsioWrapper::error_code &errc)
 {
-  ssl_listeners_.push_back(SslListener(asio::ip::tcp::acceptor(wt_.ioService()), SslConnectionPtr()));
-  asio::ip::tcp::acceptor &ssl_acceptor = ssl_listeners_.back().acceptor;
+  ssl_listeners_.push_back(std::make_shared<SslListener>(asio::ip::tcp::acceptor(wt_.ioService()), SslConnectionPtr()));
+  asio::ip::tcp::acceptor &ssl_acceptor = ssl_listeners_.back()->acceptor;
   ssl_acceptor.open(endpoint.protocol());
   ssl_acceptor.set_option(asio::ip::tcp::acceptor::reuse_address(true));
 #ifndef WT_WIN32
@@ -463,7 +463,7 @@ void Server::addSslEndpoint(const asio::ip::tcp::endpoint &endpoint,
 
     LOG_INFO_S(&wt_, "started server: " << addressString("https", endpoint, address));
 
-    ssl_listeners_.back().new_connection.reset
+    ssl_listeners_.back()->new_connection.reset
       (new SslConnection(wt_.ioService(), this, ssl_context_, connection_manager_,
                          request_handler_));
   } else {
@@ -481,13 +481,13 @@ int Server::httpPort() const
     if (ssl_listeners_.empty())
       return -1;
     else
-      return ssl_listeners_.front().acceptor.local_endpoint().port();
+      return ssl_listeners_.front()->acceptor.local_endpoint().port();
 #else // HTTP_WITH_SSL
     return -1;
 #endif // HTTP_WITH_SSL
   }
 
-  return tcp_listeners_.front().acceptor.local_endpoint().port();
+  return tcp_listeners_.front()->acceptor.local_endpoint().port();
 }
 
 void Server::startAccept()
@@ -502,23 +502,23 @@ void Server::startAccept()
    * need to access the ConnectionManager mutex in any case).
    */
   for (std::size_t i = 0; i < tcp_listeners_.size(); ++i) {
-    asio::ip::tcp::acceptor &acceptor = tcp_listeners_[i].acceptor;
-    TcpConnectionPtr &new_connection = tcp_listeners_[i].new_connection;
+    asio::ip::tcp::acceptor &acceptor = tcp_listeners_[i]->acceptor;
+    TcpConnectionPtr &new_connection = tcp_listeners_[i]->new_connection;
     acceptor.async_accept(new_connection->socket(),
                           accept_strand_.wrap(
                             std::bind(&Server::handleTcpAccept, this,
-                                        &tcp_listeners_[i],
+                                        tcp_listeners_[i],
                                         std::placeholders::_1)));
   }
 
 #ifdef HTTP_WITH_SSL
   for (std::size_t i = 0; i < ssl_listeners_.size(); ++i) {
-    asio::ip::tcp::acceptor &acceptor = ssl_listeners_[i].acceptor;
-    SslConnectionPtr &new_connection = ssl_listeners_[i].new_connection;
+    asio::ip::tcp::acceptor &acceptor = ssl_listeners_[i]->acceptor;
+    SslConnectionPtr &new_connection = ssl_listeners_[i]->new_connection;
     acceptor.async_accept(new_connection->socket(),
                           accept_strand_.wrap(
                             std::bind(&Server::handleSslAccept, this,
-                                        &ssl_listeners_[i],
+                                        ssl_listeners_[i],
                                         std::placeholders::_1)));
   }
 #endif // HTTP_WITH_SSL
@@ -539,7 +539,7 @@ void Server
   if (!err) {
     // tcp_listeners_.front() should be fine here:
     // it should be the only acceptor when this is a session process
-    auto port = tcp_listeners_.front().acceptor.local_endpoint().port();
+    auto port = tcp_listeners_.front()->acceptor.local_endpoint().port();
     Wt::WStringStream ss;
     ss << "port:" << port << "\n";
     auto buf = std::make_shared<std::string>(ss.str());
@@ -595,19 +595,20 @@ void Server::resume()
 void Server::handleResume()
 {
   for (std::size_t i = 0; i < tcp_listeners_.size(); ++i)
-    tcp_listeners_[i].acceptor.close();
+    tcp_listeners_[i]->acceptor.close();
   tcp_listeners_.clear();
 
 #ifdef HTTP_WITH_SSL
   for (std::size_t i = 0; i < ssl_listeners_.size(); ++i)
-    ssl_listeners_[i].acceptor.close();
+    ssl_listeners_[i]->acceptor.close();
   ssl_listeners_.clear();
 #endif // HTTP_WITH_SSL
 
   start();
 }
 
-void Server::handleTcpAccept(TcpListener *listener, const Wt::AsioWrapper::error_code& e)
+void Server::handleTcpAccept(const std::shared_ptr<TcpListener>& listener,
+                             const Wt::AsioWrapper::error_code& e)
 {
   if (!e) {
     connection_manager_.start(listener->new_connection);
@@ -628,7 +629,8 @@ void Server::handleTcpAccept(TcpListener *listener, const Wt::AsioWrapper::error
 }
 
 #ifdef HTTP_WITH_SSL
-void Server::handleSslAccept(SslListener *listener, const Wt::AsioWrapper::error_code& e)
+void Server::handleSslAccept(const std::shared_ptr<SslListener>& listener,
+                             const Wt::AsioWrapper::error_code& e)
 {
   if (!e) {
     connection_manager_.start(listener->new_connection);
@@ -663,12 +665,12 @@ void Server::handleStop()
   // operations. Once all operations have finished the io_service::run() call
   // will exit.
   for (std::size_t i = 0; i < tcp_listeners_.size(); ++i)
-    tcp_listeners_[i].acceptor.close();
+    tcp_listeners_[i]->acceptor.close();
   tcp_listeners_.clear();
 
 #ifdef HTTP_WITH_SSL
   for (std::size_t i = 0; i < ssl_listeners_.size(); ++i)
-    ssl_listeners_[i].acceptor.close();
+    ssl_listeners_[i]->acceptor.close();
   ssl_listeners_.clear();
 #endif // HTTP_WITH_SSL
 
diff --git a/src/http/Server.h b/src/http/Server.h
index 36758d4d8..bb6ee95d9 100644
--- a/src/http/Server.h
+++ b/src/http/Server.h
@@ -31,6 +31,8 @@
 
 #include "Wt/WLogger.h"
 
+#include <memory>
+
 namespace http {
 namespace server {
 
@@ -110,7 +112,8 @@ private:
   void closeParentConnection();
 
   /// Handle completion of an asynchronous accept operation.
-  void handleTcpAccept(TcpListener *listener, const Wt::AsioWrapper::error_code& e);
+  void handleTcpAccept(const std::shared_ptr<TcpListener>& listener,
+                       const Wt::AsioWrapper::error_code& e);
 
   /// Handle a request to stop the server.
   void handleStop();
@@ -134,7 +137,7 @@ private:
   Wt::AsioWrapper::strand accept_strand_;
 
   /// Acceptors used to listen for incoming http connections.
-  std::vector<TcpListener> tcp_listeners_;
+  std::vector<std::shared_ptr<TcpListener>> tcp_listeners_;
 
 #ifdef HTTP_WITH_SSL
   struct SslListener {
@@ -149,7 +152,7 @@ private:
   asio::ssl::context ssl_context_;
 
   /// Acceptors used to listen for incoming https connections
-  std::vector<SslListener> ssl_listeners_;
+  std::vector<std::shared_ptr<SslListener>> ssl_listeners_;
 
   /// Add new SSL listener, called from start()
   void addSslListener(asio::ip::tcp::resolver &resolver,
@@ -162,7 +165,8 @@ private:
                       Wt::AsioWrapper::error_code &errc);
 
   /// Handle completion of an asynchronous SSL accept operation.
-  void handleSslAccept(SslListener *listener, const Wt::AsioWrapper::error_code& e);
+  void handleSslAccept(const std::shared_ptr<SslListener>& listener,
+                       const Wt::AsioWrapper::error_code& e);
 #endif // HTTP_WITH_SSL
 
   void handleTimeout(asio::steady_timer *timer,
-- 
2.34.1

