[#3602] fixed crash in http/https UTs

Razvan Becheriu · Razvan Becheriu · commit d8c9dca3d1a7 · 2024-10-28T09:37:04.000+02:00
diff --git a/src/bin/d2/tests/d2_http_command_unittest.cc b/src/bin/d2/tests/d2_http_command_unittest.cc
@@ -534,9 +534,8 @@ class HttpCtrlChannelD2Test : public BaseCtrlChannelD2Test {
         response = "";
         IOServicePtr io_service = getIOService();
         ASSERT_TRUE(io_service);
-        boost::scoped_ptr<TestHttpClient> client;
-        client.reset(new TestHttpClient(io_service, SERVER_ADDRESS,
-                                        SERVER_PORT));
+        TestHttpClientPtr client(new TestHttpClient(io_service, SERVER_ADDRESS,
+                                                    SERVER_PORT));
         ASSERT_TRUE(client);
 
         // Send the command. This will trigger server's handler which
@@ -620,11 +619,11 @@ class HttpsCtrlChannelD2Test : public BaseCtrlChannelD2Test {
         response = "";
         IOServicePtr io_service = getIOService();
         ASSERT_TRUE(io_service);
-        boost::scoped_ptr<TestHttpsClient> client;
+
         TlsContextPtr client_tls_context;
         configClient(client_tls_context);
-        client.reset(new TestHttpsClient(io_service, client_tls_context,
-                                         SERVER_ADDRESS, SERVER_PORT));
+        TestHttpsClientPtr client(new TestHttpsClient(io_service, client_tls_context,
+                                                      SERVER_ADDRESS, SERVER_PORT));
         ASSERT_TRUE(client);
 
         // Send the command. This will trigger server's handler which
diff --git a/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc b/src/bin/dhcp4/tests/ctrl_dhcp4_srv_unittest.cc
@@ -132,7 +132,6 @@ class CtrlChannelDhcpv4SrvTest : public ::testing::Test {
         LeaseMgrFactory::destroy();
         StatsMgr::instance().removeAll();
 
-        UnixCommandMgr::instance().closeCommandSocket();
         CommandMgr::instance().deregisterAll();
         UnixCommandMgr::instance().setConnectionTimeout(TIMEOUT_DHCP_SERVER_RECEIVE_COMMAND);
 
@@ -532,7 +531,7 @@ TEST_F(CtrlChannelDhcpv4SrvTest, controlChannelShutdown) {
     std::string response;
 
     sendUnixCommand("{ \"command\": \"shutdown\" }", response);
-    EXPECT_EQ("{ \"result\": 0, \"text\": \"Shutting down.\" }",response);
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Shutting down.\" }", response);
 }
 
 // Tests that the server properly responds to statistics commands.  Note this
diff --git a/src/bin/dhcp4/tests/http_control_socket_unittest.cc b/src/bin/dhcp4/tests/http_control_socket_unittest.cc
@@ -107,9 +107,6 @@ class BaseCtrlChannelDhcpv4Test : public ::testing::Test {
         LeaseMgrFactory::destroy();
         StatsMgr::instance().removeAll();
 
-        if (HttpCommandMgr::instance().getHttpListener()) {
-            HttpCommandMgr::instance().close();
-        }
         CommandMgr::instance().deregisterAll();
         HttpCommandMgr::instance().setConnectionTimeout(TIMEOUT_DHCP_SERVER_RECEIVE_COMMAND);
 
@@ -623,9 +620,8 @@ class HttpCtrlChannelDhcpv4Test : public BaseCtrlChannelDhcpv4Test {
         response = "";
         IOServicePtr io_service = getIOService();
         ASSERT_TRUE(io_service);
-        boost::scoped_ptr<TestHttpClient> client;
-        client.reset(new TestHttpClient(io_service, SERVER_ADDRESS,
-                                        SERVER_PORT));
+        TestHttpClientPtr client(new TestHttpClient(io_service, SERVER_ADDRESS,
+                                                    SERVER_PORT));
         ASSERT_TRUE(client);
 
         // Send the command. This will trigger server's handler which receives
@@ -739,11 +735,11 @@ class HttpsCtrlChannelDhcpv4Test : public BaseCtrlChannelDhcpv4Test {
         response = "";
         IOServicePtr io_service = getIOService();
         ASSERT_TRUE(io_service);
-        boost::scoped_ptr<TestHttpsClient> client;
+
         TlsContextPtr client_tls_context;
         configClient(client_tls_context);
-        client.reset(new TestHttpsClient(io_service, client_tls_context,
-                                         SERVER_ADDRESS, SERVER_PORT));
+        TestHttpsClientPtr client(new TestHttpsClient(io_service, client_tls_context,
+                                                      SERVER_ADDRESS, SERVER_PORT));
         ASSERT_TRUE(client);
 
         // Send the command. This will trigger server's handler which receives
diff --git a/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc b/src/bin/dhcp6/tests/ctrl_dhcp6_srv_unittest.cc
@@ -168,7 +168,6 @@ class CtrlChannelDhcpv6SrvTest : public CtrlDhcpv6SrvTest {
         LeaseMgrFactory::destroy();
         StatsMgr::instance().removeAll();
 
-        UnixCommandMgr::instance().closeCommandSocket();
         CommandMgr::instance().deregisterAll();
         UnixCommandMgr::instance().setConnectionTimeout(TIMEOUT_DHCP_SERVER_RECEIVE_COMMAND);
 
@@ -558,7 +557,7 @@ TEST_F(CtrlChannelDhcpv6SrvTest, controlChannelShutdown) {
     std::string response;
 
     sendUnixCommand("{ \"command\": \"shutdown\" }", response);
-    EXPECT_EQ("{ \"result\": 0, \"text\": \"Shutting down.\" }",response);
+    EXPECT_EQ("{ \"result\": 0, \"text\": \"Shutting down.\" }", response);
 }
 
 // Tests that the server properly responds to statistics commands.  Note this
diff --git a/src/bin/dhcp6/tests/http_control_socket_unittest.cc b/src/bin/dhcp6/tests/http_control_socket_unittest.cc
@@ -145,9 +145,6 @@ class BaseCtrlChannelDhcpv6Test : public HttpCtrlDhcpv6Test {
         LeaseMgrFactory::destroy();
         StatsMgr::instance().removeAll();
 
-        if (HttpCommandMgr::instance().getHttpListener()) {
-            HttpCommandMgr::instance().close();
-        }
         CommandMgr::instance().deregisterAll();
         HttpCommandMgr::instance().setConnectionTimeout(TIMEOUT_DHCP_SERVER_RECEIVE_COMMAND);
 
@@ -650,9 +647,8 @@ class HttpCtrlChannelDhcpv6Test : public BaseCtrlChannelDhcpv6Test {
         response = "";
         IOServicePtr io_service = getIOService();
         ASSERT_TRUE(io_service);
-        boost::scoped_ptr<TestHttpClient> client;
-        client.reset(new TestHttpClient(io_service, SERVER_ADDRESS,
-                                        SERVER_PORT));
+        TestHttpClientPtr client(new TestHttpClient(io_service, SERVER_ADDRESS,
+                                                    SERVER_PORT));
         ASSERT_TRUE(client);
 
         // Send the command. This will trigger server's handler which receives
@@ -766,11 +762,11 @@ class HttpsCtrlChannelDhcpv6Test : public BaseCtrlChannelDhcpv6Test {
         response = "";
         IOServicePtr io_service = getIOService();
         ASSERT_TRUE(io_service);
-        boost::scoped_ptr<TestHttpsClient> client;
+
         TlsContextPtr client_tls_context;
         configClient(client_tls_context);
-        client.reset(new TestHttpsClient(io_service, client_tls_context,
-                                         SERVER_ADDRESS, SERVER_PORT));
+        TestHttpsClientPtr client(new TestHttpsClient(io_service, client_tls_context,
+                                                      SERVER_ADDRESS, SERVER_PORT));
         ASSERT_TRUE(client);
 
         // Send the command. This will trigger server's handler which receives
diff --git a/src/lib/http/listener_impl.cc b/src/lib/http/listener_impl.cc
@@ -116,7 +116,7 @@ HttpListenerImpl::accept() {
     // depends on the use case.
     HttpResponseCreatorPtr response_creator = creator_factory_->create();
     HttpAcceptorCallback acceptor_callback =
-        std::bind(&HttpListenerImpl::acceptHandler, this, ph::_1);
+        std::bind(&HttpListenerImpl::acceptHandler, shared_from_this(), ph::_1);
     HttpConnectionPtr conn = createConnection(response_creator,
                                               acceptor_callback);
     // Transmit the use external sockets flag.
diff --git a/src/lib/http/listener_impl.h b/src/lib/http/listener_impl.h
@@ -13,13 +13,14 @@
 #include <http/connection.h>
 #include <http/connection_pool.h>
 #include <http/response_creator_factory.h>
+#include <boost/enable_shared_from_this.hpp>
 #include <boost/scoped_ptr.hpp>
 
 namespace isc {
 namespace http {
 
 /// @brief Implementation of the @ref HttpListener.
-class HttpListenerImpl {
+class HttpListenerImpl : public boost::enable_shared_from_this<HttpListenerImpl> {
 public:
 
     /// @brief Constructor.
diff --git a/src/lib/http/testutils/test_http_client.h b/src/lib/http/testutils/test_http_client.h
@@ -15,6 +15,7 @@
 #include <boost/asio/read.hpp>
 #include <boost/asio/buffer.hpp>
 #include <boost/asio/ip/tcp.hpp>
+#include <boost/enable_shared_from_this.hpp>
 #include <gtest/gtest.h>
 
 using namespace boost::asio::ip;
@@ -85,7 +86,8 @@ class BaseTestHttpClient : public boost::noncopyable {
 };
 
 /// @brief Entity which can connect to the HTTP server endpoint.
-class TestHttpClient : public BaseTestHttpClient {
+class TestHttpClient : public BaseTestHttpClient,
+                       public boost::enable_shared_from_this<TestHttpClient> {
 public:
 
     /// @brief Constructor.
@@ -116,8 +118,9 @@ class TestHttpClient : public BaseTestHttpClient {
     /// @param request HTTP request in the textual format.
     virtual void startRequest(const std::string& request) {
         tcp::endpoint endpoint(address::from_string(server_address_), server_port_);
+        auto ref = shared_from_this();
         socket_.async_connect(endpoint,
-        [this, request](const boost::system::error_code& ec) {
+                              [this, ref, request](const boost::system::error_code& ec) {
             receive_done_ = false;
             if (ec) {
                 // One would expect that async_connect wouldn't return
@@ -150,9 +153,10 @@ class TestHttpClient : public BaseTestHttpClient {
     /// @param request part of the HTTP request to be sent.
     virtual void sendPartialRequest(std::string request) {
         size_t chuck_size = std::min(TEST_HTTP_CHUCK_SIZE, request.size());
+        auto ref = shared_from_this();
         socket_.async_send(boost::asio::buffer(request.data(), chuck_size),
-                           [this, request](const boost::system::error_code& ec,
-                                           std::size_t bytes_transferred) mutable {
+                           [this, ref, request](const boost::system::error_code& ec,
+                                                std::size_t bytes_transferred) mutable {
             if (ec) {
                 if (ec.value() == boost::asio::error::operation_aborted) {
                     return;
@@ -189,9 +193,10 @@ class TestHttpClient : public BaseTestHttpClient {
 
     /// @brief Receive response from the server.
     virtual void receivePartialResponse() {
-        socket_.async_read_some(boost::asio::buffer(buf_.data(), buf_.size()),
-                                [this](const boost::system::error_code& ec,
-                                       std::size_t bytes_transferred) {
+        auto ref = shared_from_this();
+        socket_.async_read_some(boost::asio::buffer(ref->buf_.data(), ref->buf_.size()),
+                                [this, ref](const boost::system::error_code& ec,
+                                            std::size_t bytes_transferred) {
             if (ec) {
                 // IO service stopped so simply return.
                 if (ec.value() == boost::asio::error::operation_aborted) {
@@ -354,7 +359,8 @@ class TestHttpClient : public BaseTestHttpClient {
 typedef boost::shared_ptr<TestHttpClient> TestHttpClientPtr;
 
 /// @brief Entity which can connect to the HTTPS server endpoint.
-class TestHttpsClient : public BaseTestHttpClient {
+class TestHttpsClient : public BaseTestHttpClient,
+                        public boost::enable_shared_from_this<TestHttpsClient> {
 public:
 
     /// @brief Constructor.
@@ -388,8 +394,9 @@ class TestHttpsClient : public BaseTestHttpClient {
     virtual void startRequest(const std::string& request) {
         tcp::endpoint endpoint(address::from_string(server_address_),
                                server_port_);
+        auto ref = shared_from_this();
         stream_.lowest_layer().async_connect(endpoint,
-        [this, request](const boost::system::error_code& ec) {
+                                             [this, ref, request](const boost::system::error_code& ec) {
             receive_done_ = false;
             if (ec) {
                 // One would expect that async_connect wouldn't return
@@ -431,10 +438,11 @@ class TestHttpsClient : public BaseTestHttpClient {
     /// @param request part of the HTTP request to be sent.
     virtual void sendPartialRequest(std::string request) {
         size_t chuck_size = std::min(TEST_HTTP_CHUCK_SIZE, request.size());
+        auto ref = shared_from_this();
         boost::asio::async_write(stream_,
-                boost::asio::buffer(request.data(), chuck_size),
-                [this, request](const boost::system::error_code& ec,
-                                std::size_t bytes_transferred) mutable {
+                                 boost::asio::buffer(request.data(), chuck_size),
+                                 [this, ref, request](const boost::system::error_code& ec,
+                                                      std::size_t bytes_transferred) mutable {
             if (ec) {
                 if (ec.value() == boost::asio::error::operation_aborted) {
                     return;
@@ -471,9 +479,10 @@ class TestHttpsClient : public BaseTestHttpClient {
 
     /// @brief Receive response from the server.
     virtual void receivePartialResponse() {
-        stream_.async_read_some(boost::asio::buffer(buf_.data(), buf_.size()),
-                                [this](const boost::system::error_code& ec,
-                                       std::size_t bytes_transferred) {
+        auto ref = shared_from_this();
+        stream_.async_read_some(boost::asio::buffer(ref->buf_.data(), ref->buf_.size()),
+                                [this, ref](const boost::system::error_code& ec,
+                                            std::size_t bytes_transferred) {
             if (ec) {
                 // IO service stopped so simply return.
                 if (ec.value() == boost::asio::error::operation_aborted) {
