From 9453a7cc558c3306168473da22d639eae4c6af94 Mon Sep 17 00:00:00 2001 From: tqcq Date: Mon, 1 Jul 2024 01:47:54 +0000 Subject: [PATCH] fix on_init crash (#3) Reviewed-on: https://code.uocat.com/tqcq/tile/pulls/3 --- CMakeLists.txt | 11 ++-- tests/http_client_test.cc | 85 +++++++++++++++++++++++++++++++ tests/http_server.py | 24 +++++++++ tile/base/make_unique.h | 25 ++++++++- tile/init/on_init.cc | 1 + tile/net/http/http_client.cc | 11 +++- tile/net/http/http_client.h | 3 ++ tile/net/http/http_client_test.cc | 6 +-- tile/net/http/types.h | 22 ++++++++ tile/net/internal/http_engine.cc | 4 ++ 10 files changed, 183 insertions(+), 9 deletions(-) create mode 100644 tests/http_client_test.cc create mode 100644 tests/http_server.py diff --git a/CMakeLists.txt b/CMakeLists.txt index 6a83176..19836bb 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -268,17 +268,21 @@ if(TILE_BUILD_TESTS) add_executable(tile_test_all "tile/testing/main.cc") target_link_libraries(tile_test_all PRIVATE gtest gmock tile::tile) - - macro(tile_add_test test_name test_file) + + macro(tile_add_custom_test test_name test_file) add_executable(${test_name} ${test_file} "tile/testing/main.cc") target_link_libraries( ${test_name} PUBLIC gtest gmock ${WHOLE_ARCHIVE_PREFIX} tile::tile ${WHOLE_ARCHIVE_SUFFIX}) - add_test(NAME ${test_name} COMMAND ${test_name}) target_sources(${PROJECT_NAME}_test_all PRIVATE ${test_file}) endmacro() + macro(tile_add_test test_name test_file) + tile_add_custom_test(${test_name} ${test_file}) + add_test(NAME ${test_name} COMMAND ${test_name}) + endmacro() + tile_add_test(fiber_detail_scheduler_test "tile/fiber/detail/scheduler_test.cc") tile_add_test(base_internal_meta_test "tile/base/internal/meta_test.cc") # tile_add_test(net_internal_http_engine_test @@ -348,6 +352,7 @@ tile_add_test(fiber_detail_scheduler_test "tile/fiber/detail/scheduler_test.cc") tile_add_test(base_internal_logging_test "tile/base/internal/logging_test.cc") tile_add_test(base_chrono_test "tile/base/chrono_test.cc") tile_add_test(init_override_flag_test "tile/init/override_flag_test.cc") + tile_add_custom_test("custom_http_client_test" "tests/http_client_test.cc") # tile_add_test(base_internal_time_keeper_test # "tile/base/internal/time_keeper_test.cc") endif(TILE_BUILD_TESTS) diff --git a/tests/http_client_test.cc b/tests/http_client_test.cc new file mode 100644 index 0000000..d9407cf --- /dev/null +++ b/tests/http_client_test.cc @@ -0,0 +1,85 @@ +#include "tile/net/http/http_client.h" + +#include "gtest/gtest.h" + +#include "tile/base/thread/latch.h" + +const std::string url = "http://127.0.0.1:8000/"; + +TEST(HttpClient, Request) { + tile::HttpClient client; + std::vector>> + v; + tile::HttpClient::RequestOptions options; + options.timeout = std::chrono::seconds(20); + + std::atomic count{0}; + + for (int i = 0; i != 1000; ++i) { + for (auto code : tile::AllHttpStatus) { + if (static_cast(code) < 200) { + continue; + } + + v.emplace_back( + client.AsyncGet(url + std::to_string(static_cast(code)), options) + .Then([&](std::expected &&res) { + int cnt = count.fetch_add(1) + 1; + TILE_LOG_INFO_EVERY_SECOND("complete {}", cnt); + return res; + })); + } + } + + int j = 0; + for (int i = 0; i != 1000; ++i) { + for (auto &code : tile::AllHttpStatus) { + if (static_cast(code) < 200) { + continue; + } + + auto resp = tile::future::BlockingGet(std::move(v[j++])); + + EXPECT_TRUE(resp) << tile::HttpClient::ErrorCodeToString(resp.error()); + EXPECT_EQ(resp->status(), code); + } + } +} + +TEST(HttpClient, RequestNoWait) { + tile::HttpClient client; + tile::HttpClient::RequestOptions options; + options.timeout = std::chrono::seconds(20); + int cnt = 0; + + for (int i = 0; i != 1000; ++i) { + for (auto code : tile::AllHttpStatus) { + if (static_cast(code) < 200) { + continue; + } + ++cnt; + } + } + + tile::Latch latch(cnt); + std::atomic count{0}; + + for (int i = 0; i != 1000; ++i) { + for (auto code : tile::AllHttpStatus) { + if (static_cast(code) < 200) { + continue; + } + + client.AsyncGet(url + std::to_string(static_cast(code)), options) + .Then([&](std::expected &&) { + int cnt = count.fetch_add(1) + 1; + TILE_LOG_INFO_EVERY_SECOND("complete {}", cnt); + latch.CountDown(); + }); + } + } + latch.Wait(); +} diff --git a/tests/http_server.py b/tests/http_server.py new file mode 100644 index 0000000..446730c --- /dev/null +++ b/tests/http_server.py @@ -0,0 +1,24 @@ +#!/usr/bin/python3 +import http.server +import socket +import socketserver +from http.server import HTTPServer, BaseHTTPRequestHandler + +PORT = 8000 + +class Request(BaseHTTPRequestHandler): + def do_GET(self): + code = int(self.path.split("/")[1]) + + self.send_response(code) + self.send_header('Content-type', 'text/plain') + self.end_headers() + +class HTTPServer(socketserver.TCPServer): + def server_bind(self): + self.socket.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + self.socket.bind(self.server_address) + +Handler = http.server.SimpleHTTPRequestHandler +with HTTPServer(("", PORT), Request) as httpd: + httpd.serve_forever() diff --git a/tile/base/make_unique.h b/tile/base/make_unique.h index 6f1eaac..d6d270b 100644 --- a/tile/base/make_unique.h +++ b/tile/base/make_unique.h @@ -21,7 +21,7 @@ make_unique(Args &&...args) { template inline enable_if_t::value && std::extent::value == 0, std::unique_ptr> -make_unique(size_t size) { +make_unique(std::size_t size) { TILE_DCHECK(size > 0); using U = typename std::remove_extent::type; return std::unique_ptr(new U[size]()); @@ -34,6 +34,29 @@ enable_if_t::value && std::extent::value != 0, std::unique_ptr> make_unique(Args &&...) = delete; +// == With Deleter + +template +inline enable_if_t::value, std::unique_ptr> +make_unique_with_deleter(D &&deleter, Args &&...args) { + return std::unique_ptr(new T(std::forward(args)...), + std::forward(deleter)); +} + +template +inline enable_if_t::value && std::extent::value == 0, + std::unique_ptr> +make_unique_with_deleter(D &&deleter, std::size_t size) { + TILE_DCHECK(size > 0); + using U = typename std::remove_extent::type; + return std::unique_ptr(new U[size](), std::forward(deleter)); +} + +template +enable_if_t::value && std::extent::value != 0, + std::unique_ptr> +make_unique_with_deleter(D &&deleter, Args &&...) = delete; + } // namespace tile #endif // TILE_BASE_MAKE_UNIQUE_H diff --git a/tile/init/on_init.cc b/tile/init/on_init.cc index 7f0fbfa..3ad04b0 100644 --- a/tile/init/on_init.cc +++ b/tile/init/on_init.cc @@ -83,6 +83,7 @@ void RegisterOnInitCallback(std::int32_t priority, std::function init, !registry_prepared.load(std::memory_order_relaxed), "Callbacks may only be registered before `tile::Start()` is called"); auto &®istry = *GetStagingRegistry(); + TILE_CHECK(init != nullptr, "Initializer must be provided"); registry[priority].push_back({std::move(init), std::move(fini)}); } diff --git a/tile/net/http/http_client.cc b/tile/net/http/http_client.cc index 924500a..ade386f 100644 --- a/tile/net/http/http_client.cc +++ b/tile/net/http/http_client.cc @@ -49,10 +49,15 @@ HttpClient::ErrorCode GetErrorCodeFromCurlCode(int c) { return HttpClient::ERROR_IO; case CURLE_TOO_MANY_REDIRECTS: return HttpClient::ERROR_TOO_MANY_REDIRECTS; - default: - TILE_LOG_WARNING_EVERY_SECOND("ERROR_UNKNOWN CURLcode {}", c); + case CURLE_GOT_NOTHING: + return HttpClient::ERROR_GET_NOTHING; + + default: { + TILE_LOG_WARNING_EVERY_SECOND("ERROR_UNKNOWN CURLcode {}, curl msg: ", c, + curl_easy_strerror(static_cast(c))); return HttpClient::ERROR_UNKNOWN; } + } } long TileHttpVersionToCurlHttpVersion(HttpVersion v, @@ -446,6 +451,8 @@ const char *HttpClient::ErrorCodeToString(int error_code) { return "ERROR_INTERNAL_ERROR"; case ERROR_DRY_RUN: return "ERROR_DRY_RUN"; + case ERROR_GET_NOTHING: + return "ERROR_GET_NOTHING"; case ERROR_UNKNOWN: return ""; } diff --git a/tile/net/http/http_client.h b/tile/net/http/http_client.h index f9057f7..77eea63 100644 --- a/tile/net/http/http_client.h +++ b/tile/net/http/http_client.h @@ -11,6 +11,8 @@ #include "gflags/gflags_declare.h" +#include + DECLARE_int32(tile_http_client_default_timeout_ms); namespace tile { @@ -45,6 +47,7 @@ public: ERROR_IO, ERROR_INTERNAL_ERROR, ERROR_DRY_RUN, + ERROR_GET_NOTHING, ERROR_UNKNOWN = 100, }; diff --git a/tile/net/http/http_client_test.cc b/tile/net/http/http_client_test.cc index 9a8fef3..fa56d23 100644 --- a/tile/net/http/http_client_test.cc +++ b/tile/net/http/http_client_test.cc @@ -24,7 +24,7 @@ Get(const std::string &url, std::chrono::nanoseconds timeout = 20 * one_s) { bool IsWanAccessible() { static const auto kResult = /*!!Get("http://baidu.com") && !!Get("http://qq.com") &&*/ - !!Get("http://example.com"); + !!Get("http://www.baidu.com"); return kResult; } @@ -43,7 +43,7 @@ TEST(HttpClient, TestDomain) { TILE_LOG_INFO("WAN is not accessible, skip this test"); return; } - auto resp = Get("http://example.com"); + auto resp = Get("http://www.baidu.com"); ASSERT_TRUE(resp); EXPECT_EQ(HttpStatus(200), resp->status()); } @@ -53,7 +53,7 @@ TEST(HttpClient, TestNotFound) { TILE_LOG_INFO("WAN is not accessible, skip this test"); return; } - auto resp = Get("http://example.com/404"); + auto resp = Get("http://www.baidu.com/404"); ASSERT_TRUE(resp); EXPECT_EQ(HttpStatus(404), resp->status()); } diff --git a/tile/net/http/types.h b/tile/net/http/types.h index 7a9aa91..317181e 100644 --- a/tile/net/http/types.h +++ b/tile/net/http/types.h @@ -79,6 +79,28 @@ enum class HttpStatus { // `HttpStatusCode`? NetworkAuthenticationRequired = 511 }; +// AllHttpStatus for for_each +static HttpStatus AllHttpStatus[] = { + HttpStatus::Continue, + HttpStatus::SwitchingProtocols, + HttpStatus::EarlyHints, + HttpStatus::OK, + HttpStatus::Created, + HttpStatus::Accepted, + HttpStatus::NonAuthoritativeInformation, + HttpStatus::NoContent, + HttpStatus::ResetContent, + HttpStatus::PartialContent, + HttpStatus::MultipleChoices, + HttpStatus::MovedPermanently, + HttpStatus::Found, + HttpStatus::SeeOther, + HttpStatus::NotModified, + HttpStatus::TemporaryRedirect, + HttpStatus::PermanentRedirect, + HttpStatus::BadRequest, +}; + Slice ToSlice(HttpVersion method) noexcept; Slice ToSlice(HttpMethod) noexcept; Slice ToSlice(HttpStatus status) noexcept; diff --git a/tile/net/internal/http_engine.cc b/tile/net/internal/http_engine.cc index 4192f32..2b12a2c 100644 --- a/tile/net/internal/http_engine.cc +++ b/tile/net/internal/http_engine.cc @@ -267,6 +267,10 @@ public: for (auto &&c : clients_) { c->Stop(); } + + for (auto &&c : clients_) { + c.reset(); + } } private: