From d49be23aabc5b50f09f71882fefde533163d8939 Mon Sep 17 00:00:00 2001
From: qicosmos <qicosmos@linux.alibaba.com>
Date: Wed, 5 Feb 2025 16:03:14 +0800
Subject: [PATCH] [coro_http]fix data race (#892)

fix data race
---
 .../standalone/cinatra/coro_http_client.hpp   |  2 +-
 .../standalone/cinatra/coro_http_server.hpp   | 17 ++++++++----
 .../standalone/cinatra/session_manager.hpp    | 26 ++++++++++---------
 src/coro_http/tests/test_cinatra.cpp          |  4 +--
 .../tests/test_cinatra_websocket.cpp          |  2 +-
 5 files changed, 30 insertions(+), 21 deletions(-)

diff --git a/include/ylt/standalone/cinatra/coro_http_client.hpp b/include/ylt/standalone/cinatra/coro_http_client.hpp
index 52876fadc..bc9018844 100644
--- a/include/ylt/standalone/cinatra/coro_http_client.hpp
+++ b/include/ylt/standalone/cinatra/coro_http_client.hpp
@@ -1476,7 +1476,7 @@ class coro_http_client : public std::enable_shared_from_this<coro_http_client> {
   struct socket_t {
     asio::ip::tcp::socket impl_;
     std::atomic<bool> has_closed_ = true;
-    bool is_timeout_ = false;
+    std::atomic<bool> is_timeout_ = false;
     asio::streambuf head_buf_;
     asio::streambuf chunked_buf_;
 #ifdef CINATRA_ENABLE_SSL
diff --git a/include/ylt/standalone/cinatra/coro_http_server.hpp b/include/ylt/standalone/cinatra/coro_http_server.hpp
index 3b3f81345..6fb345e1a 100644
--- a/include/ylt/standalone/cinatra/coro_http_server.hpp
+++ b/include/ylt/standalone/cinatra/coro_http_server.hpp
@@ -88,6 +88,7 @@ class coro_http_server {
 
     if (!errc_) {
       if (out_ctx_ == nullptr) {
+        std::lock_guard lock(thd_mtx_);
         thd_ = std::thread([this] {
           pool_->run();
         });
@@ -112,13 +113,14 @@ class coro_http_server {
 
   // only call once, not thread safe.
   void stop() {
-    if (out_ctx_ == nullptr && !thd_.joinable()) {
-      return;
+    {
+      std::lock_guard lock(thd_mtx_);
+      if (out_ctx_ == nullptr && !thd_.joinable()) {
+        return;
+      }
     }
 
     stop_timer_ = true;
-    std::error_code ec;
-    check_timer_.cancel(ec);
 
     close_acceptor();
 
@@ -136,7 +138,11 @@ class coro_http_server {
       pool_->stop();
 
       CINATRA_LOG_INFO << "server's thread-pool finished.";
-      thd_.join();
+      {
+        std::lock_guard lock(thd_mtx_);
+        thd_.join();
+      }
+
       CINATRA_LOG_INFO << "stop coro_http_server ok";
     }
     else {
@@ -977,6 +983,7 @@ class coro_http_server {
   std::error_code errc_ = {};
   asio::ip::tcp::acceptor acceptor_;
   std::thread thd_;
+  std::mutex thd_mtx_;
   std::promise<void> acceptor_close_waiter_;
   bool no_delay_ = true;
 
diff --git a/include/ylt/standalone/cinatra/session_manager.hpp b/include/ylt/standalone/cinatra/session_manager.hpp
index 2178ed92f..29b1ea412 100644
--- a/include/ylt/standalone/cinatra/session_manager.hpp
+++ b/include/ylt/standalone/cinatra/session_manager.hpp
@@ -60,8 +60,14 @@ class session_manager {
   }
 
   void start_check_session_timer() {
-    check_session_timer_.expires_after(check_session_duration_);
-    check_session_timer_.async_wait([this](auto ec) {
+    std::weak_ptr<asio::steady_timer> timer = check_session_timer_;
+    check_session_timer_->expires_after(check_session_duration_);
+    check_session_timer_->async_wait([this, timer](auto ec) {
+      auto ptr = timer.lock();
+      if (ptr == nullptr) {
+        return;
+      }
+
       if (ec || stop_timer_) {
         return;
       }
@@ -76,16 +82,12 @@ class session_manager {
     start_check_session_timer();
   }
 
-  void stop_timer() {
-    stop_timer_ = true;
-    std::error_code ec;
-    check_session_timer_.cancel(ec);
-  }
+  void stop_timer() { stop_timer_ = true; }
 
  private:
   session_manager()
-      : check_session_timer_(
-            coro_io::get_global_executor()->get_asio_executor()) {
+      : check_session_timer_(std::make_shared<asio::steady_timer>(
+            coro_io::get_global_executor()->get_asio_executor())) {
     start_check_session_timer();
   };
   session_manager(const session_manager &) = delete;
@@ -98,9 +100,9 @@ class session_manager {
   // session_timeout_ should be no less than 0
   std::size_t session_timeout_ = 86400;
   std::atomic<bool> stop_timer_ = false;
-  asio::steady_timer check_session_timer_;
-  std::chrono::steady_clock::duration check_session_duration_ =
-      std::chrono::seconds(15);
+  std::shared_ptr<asio::steady_timer> check_session_timer_;
+  std::atomic<std::chrono::steady_clock::duration> check_session_duration_ = {
+      std::chrono::seconds(15)};
 };
 
 }  // namespace cinatra
\ No newline at end of file
diff --git a/src/coro_http/tests/test_cinatra.cpp b/src/coro_http/tests/test_cinatra.cpp
index a532e19d4..31c2340a0 100644
--- a/src/coro_http/tests/test_cinatra.cpp
+++ b/src/coro_http/tests/test_cinatra.cpp
@@ -2706,12 +2706,12 @@ TEST_CASE("test coro http redirect request") {
   if (result.status != 404 && !result.net_err) {
     CHECK(!result.net_err);
     if (result.status < 500)
-      CHECK(result.status == 302);
+      CHECK((result.status == 302 || result.status == 301));
 
     if (client.is_redirect(result)) {
       std::string redirect_uri = client.get_redirect_uri();
       result = async_simple::coro::syncAwait(client.async_get(redirect_uri));
-      if (result.status < 400)
+      if (result.status < 300)
         CHECK(result.status == 200);
     }
 
diff --git a/src/coro_http/tests/test_cinatra_websocket.cpp b/src/coro_http/tests/test_cinatra_websocket.cpp
index 21effcd1b..12108c907 100644
--- a/src/coro_http/tests/test_cinatra_websocket.cpp
+++ b/src/coro_http/tests/test_cinatra_websocket.cpp
@@ -330,7 +330,7 @@ TEST_CASE("test read write in different threads") {
     }
   };
 
-  async_simple::coro::syncAwait(lazy());
+  async_simple::coro::syncAwait(lazy().via(&client->get_executor()));
 
   promise.get_future().wait_for(std::chrono::seconds(2));