From dab7d9e3fb574b4722c5ce071c6e56b1255b4eef Mon Sep 17 00:00:00 2001 From: jbaldwin Date: Wed, 20 Dec 2023 14:55:21 -0700 Subject: [PATCH] Add test to verify dup'ed sockets can poll on read/write This change adds a test to prove that duplicating a tcp::client allows you to read in one coroutine while writing in another. This does not allow you to read in two coroutines, the epoll_ctl_mod will still fail. Closes #224 --- .github/workflows/ci-coverage.yml | 4 +- .github/workflows/ci-emscripten.yml | 2 +- .github/workflows/ci-fedora.yml | 8 +-- .github/workflows/ci-opensuse.yml | 4 +- .github/workflows/ci-ubuntu.yml | 4 +- CMakeLists.txt | 12 +++-- include/coro/io_scheduler.hpp | 12 +---- src/io_scheduler.cpp | 12 +++++ test/CMakeLists.txt | 34 +++++++----- test/net/test_tcp_server.cpp | 84 +++++++++++++++++++++++++++++ test/net/test_tls_server.cpp | 2 +- 11 files changed, 140 insertions(+), 38 deletions(-) diff --git a/.github/workflows/ci-coverage.yml b/.github/workflows/ci-coverage.yml index 951c548b..40961f58 100644 --- a/.github/workflows/ci-coverage.yml +++ b/.github/workflows/ci-coverage.yml @@ -44,11 +44,11 @@ jobs: -DLIBCORO_FEATURE_TLS=ON \ -DLIBCORO_CODE_COVERAGE=ON \ .. - ninja + cmake --build . --config Debug - name: Coverage run: | cd Debug - ctest -VV + ctest --build-config Debug -VV gcov -o ./test/CMakeFiles/libcoro_test.dir/main.cpp.o ./test/libcoro_test lcov --include "*/include/coro/*" --include "*/src/*" --exclude "test/*" -o libcoro_tests.lcov -c -d . - name: Coveralls GitHub Action diff --git a/.github/workflows/ci-emscripten.yml b/.github/workflows/ci-emscripten.yml index 413c59d0..6b98565f 100644 --- a/.github/workflows/ci-emscripten.yml +++ b/.github/workflows/ci-emscripten.yml @@ -44,7 +44,7 @@ jobs: -GNinja \ -DCMAKE_BUILD_TYPE=Release \ .. - ninja + cmake --build . --config Release - name: Test run: | cd emsdk diff --git a/.github/workflows/ci-fedora.yml b/.github/workflows/ci-fedora.yml index a831bf13..041eefcc 100644 --- a/.github/workflows/ci-fedora.yml +++ b/.github/workflows/ci-fedora.yml @@ -9,8 +9,8 @@ jobs: strategy: fail-fast: false matrix: - fedora_version: [32, 33, 34, 35, 36, 37] - libcoro_feature_networking: [ {enabled: ON, tls: ON}] + fedora_version: [32, 33, 34, 35, 36, 37, 38, 39, 40] + libcoro_feature_networking: [ {enabled: ON, tls: ON} ] libcoro_feature_platform: [ON] container: image: fedora:${{ matrix.fedora_version }} @@ -41,8 +41,8 @@ jobs: -DLIBCORO_FEATURE_TLS=${{ matrix.libcoro_feature_networking.tls }} \ -DLIBCORO_FEATURE_PLATFORM=${{ matrix.libcoro_feature_platform }} \ .. - ninja + cmake --build . --config Release - name: Test run: | cd Release - ctest -VV + ctest --build-config Release -VV diff --git a/.github/workflows/ci-opensuse.yml b/.github/workflows/ci-opensuse.yml index f037dad6..6ce169c3 100644 --- a/.github/workflows/ci-opensuse.yml +++ b/.github/workflows/ci-opensuse.yml @@ -42,9 +42,9 @@ jobs: -DLIBCORO_FEATURE_TLS=${{ matrix.libcoro_feature_networking.tls }} \ -DLIBCORO_FEATURE_PLATFORM=${{ matrix.libcoro_feature_platform }} \ .. - ninja + cmake --build . --config Release - name: Test run: | cd Release - ctest -VV + ctest --build-config Release -VV diff --git a/.github/workflows/ci-ubuntu.yml b/.github/workflows/ci-ubuntu.yml index 6832b5a3..5c951c4d 100644 --- a/.github/workflows/ci-ubuntu.yml +++ b/.github/workflows/ci-ubuntu.yml @@ -146,8 +146,8 @@ jobs: -DLIBCORO_FEATURE_TLS=${{ matrix.libcoro_feature_networking.tls }} \ -DLIBCORO_FEATURE_PLATFORM=${{ matrix.libcoro_feature_platform }} \ .. - ninja + cmake --build . --config Release - name: Test run: | cd Release - ctest -VV + ctest --build-config Release -VV diff --git a/CMakeLists.txt b/CMakeLists.txt index c56bbe11..f878885b 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -150,7 +150,7 @@ endif() if(${CMAKE_CXX_COMPILER_ID} MATCHES "GNU") if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "10.2.0") - message(FATAL_ERROR "gcc version ${CMAKE_CXX_COMPILER_VERSION} is unsupported, please upgrade to at least 10.2.0") + message(FATAL_ERROR "g++ version ${CMAKE_CXX_COMPILER_VERSION} is unsupported, please upgrade to at least 10.2.0") endif() target_compile_options(${PROJECT_NAME} PUBLIC @@ -160,7 +160,8 @@ if(${CMAKE_CXX_COMPILER_ID} MATCHES "GNU") $<$:-fexceptions> $<$:-Wall> $<$:-Wextra> - $<$:-pipe>) + $<$:-pipe> + ) elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang") if(CMAKE_CXX_COMPILER_VERSION VERSION_LESS "16.0.0") message(FATAL_ERROR "Clang version ${CMAKE_CXX_COMPILER_VERSION} is unsupported, please upgrade to at least 16.0.0") @@ -171,9 +172,12 @@ elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang") $<$:-fexceptions> $<$:-Wall> $<$:-Wextra> - $<$:-pipe>) + $<$:-pipe> + ) elseif(MSVC) - target_compile_options(${PROJECT_NAME} PUBLIC /W4) + target_compile_options(${PROJECT_NAME} PUBLIC + /W4 + ) endif() if(LIBCORO_BUILD_TESTS) diff --git a/include/coro/io_scheduler.hpp b/include/coro/io_scheduler.hpp index 9aca57bf..81ec4495 100644 --- a/include/coro/io_scheduler.hpp +++ b/include/coro/io_scheduler.hpp @@ -162,11 +162,7 @@ class io_scheduler * @param task The task to execute on this io_scheduler. It's lifetime ownership will be transferred * to this io_scheduler. */ - auto schedule(coro::task&& task) -> void - { - auto* ptr = static_cast*>(m_owned_tasks); - ptr->start(std::move(task)); - } + auto schedule(coro::task&& task) -> void; /** * Schedules the current task to run after the given amount of time has elapsed. @@ -290,11 +286,7 @@ class io_scheduler * new tasks but this allows the user to cleanup resources manually. One usage might be making sure fds * are cleaned up as soon as possible. */ - auto garbage_collect() noexcept -> void - { - auto* ptr = static_cast*>(m_owned_tasks); - ptr->garbage_collect(); - } + auto garbage_collect() noexcept -> void; private: /// The configuration options. diff --git a/src/io_scheduler.cpp b/src/io_scheduler.cpp index 603d0f94..e639bd82 100644 --- a/src/io_scheduler.cpp +++ b/src/io_scheduler.cpp @@ -84,6 +84,12 @@ auto io_scheduler::process_events(std::chrono::milliseconds timeout) -> std::siz return size(); } +auto io_scheduler::schedule(coro::task&& task) -> void +{ + auto* ptr = static_cast*>(m_owned_tasks); + ptr->start(std::move(task)); +} + auto io_scheduler::schedule_after(std::chrono::milliseconds amount) -> coro::task { return yield_for(amount); @@ -202,6 +208,12 @@ auto io_scheduler::shutdown() noexcept -> void } } +auto io_scheduler::garbage_collect() noexcept -> void +{ + auto* ptr = static_cast*>(m_owned_tasks); + ptr->garbage_collect(); +} + auto io_scheduler::process_events_manual(std::chrono::milliseconds timeout) -> void { bool expected{false}; diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index c143e801..e6cae05d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -35,33 +35,43 @@ endif() if(LIBCORO_FEATURE_PLATFORM) list(APPEND LIBCORO_TEST_SOURCE_FILES - bench.cpp # networking is ifdef'ed internally + bench.cpp # internally ifdef's LIBCORO_FEATURE_NETWORKING|TLS test_io_scheduler.cpp ) endif() add_executable(${PROJECT_NAME} main.cpp ${LIBCORO_TEST_SOURCE_FILES}) -target_compile_features(${PROJECT_NAME} PUBLIC cxx_std_20) +target_compile_features(${PROJECT_NAME} PRIVATE cxx_std_20) target_include_directories(${PROJECT_NAME} PRIVATE ${CMAKE_CURRENT_SOURCE_DIR}) - -set(LIBCORO_TEST_LINK_LIBRARIES libcoro) +target_link_libraries(${PROJECT_NAME} PRIVATE libcoro) if(${CMAKE_CXX_COMPILER_ID} MATCHES "GNU") - set(LIBCORO_TEST_COMPILE_OPTIONS -fcoroutines -Wall -Wextra -pipe) + target_compile_options(${PROJECT_NAME} PRIVATE + $<$:-std=c++20> + $<$:-fcoroutines> + $<$:-fconcepts> + $<$:-fexceptions> + $<$:-Wall> + $<$:-Wextra> + $<$:-pipe> + ) elseif(${CMAKE_CXX_COMPILER_ID} MATCHES "Clang") - set(LIBCORO_TEST_COMPILE_OPTIONS -Wall -Wextra -pipe) + target_compile_options(${PROJECT_NAME} PRIVATE + $<$:-std=c++20> + $<$:-fexceptions> + $<$:-Wall> + $<$:-Wextra> + $<$:-pipe> + ) elseif(MSVC) - set(LIBCORO_TEST_COMPILE_OPTIONS /W4) + target_compile_options(${PROJECT_NAME} PRIVATE /W4) else() message(FATAL_ERROR "Unsupported compiler.") endif() if(LIBCORO_CODE_COVERAGE) - list(APPEND LIBCORO_TEST_LINK_LIBRARIES gcov) - list(APPEND LIBCORO_TEST_COMPILE_OPTIONS --coverage) + target_link_libraries(${PROJECT_NAME} PRIVATE gcov) + target_compile_options(${PROJECT_NAME} PRIVATE --coverage) endif() -target_link_libraries(${PROJECT_NAME} PUBLIC ${LIBCORO_TEST_LINK_LIBRARIES}) -target_compile_options(${PROJECT_NAME} PUBLIC ${LIBCORO_TEST_COMPILE_OPTIONS}) - add_test(NAME libcoro_tests COMMAND ${PROJECT_NAME}) diff --git a/test/net/test_tcp_server.cpp b/test/net/test_tcp_server.cpp index 78f08138..33ebebf7 100644 --- a/test/net/test_tcp_server.cpp +++ b/test/net/test_tcp_server.cpp @@ -86,4 +86,88 @@ TEST_CASE("tcp_server ping server", "[tcp_server]") coro::sync_wait(coro::when_all(make_server_task(), make_client_task())); } +TEST_CASE("tcp_server concurrent polling on the same socket", "[tcp_server]") +{ + // Issue 224: This test duplicates a client and issues two different poll operations per coroutine. + + using namespace std::chrono_literals; + auto scheduler = std::make_shared(coro::io_scheduler::options{ + .execution_strategy = coro::io_scheduler::execution_strategy_t::process_tasks_inline}); + + auto make_read_task = [](coro::net::tcp::client client) -> coro::task + { + co_await client.poll(coro::poll_op::read, 2s); + co_return; + }; + + auto make_server_task = [&]() -> coro::task + { + co_await scheduler->schedule(); + coro::net::tcp::server server{scheduler}; + + auto poll_status = co_await server.poll(); + REQUIRE(poll_status == coro::poll_status::event); + + auto read_client = server.accept(); + REQUIRE(read_client.socket().is_valid()); + + // make a copy so we can poll twice at the same time in different coroutines + auto write_client = read_client; + + scheduler->schedule(make_read_task(std::move(read_client))); + + // Make sure the read op has completed. + co_await scheduler->yield_for(500ms); + + std::string data(8096, 'A'); + std::span remaining{data}; + do + { + auto poll_status = co_await write_client.poll(coro::poll_op::write); + REQUIRE(poll_status == coro::poll_status::event); + auto [send_status, r] = write_client.send(remaining); + REQUIRE(send_status == coro::net::send_status::ok); + + if (r.empty()) + { + break; + } + + remaining = r; + } while (true); + + co_return data; + }; + + auto make_client_task = [&]() -> coro::task + { + co_await scheduler->schedule(); + coro::net::tcp::client client{scheduler}; + + auto connect_status = co_await client.connect(); + REQUIRE(connect_status == coro::net::connect_status::connected); + + std::string response(8096, '\0'); + std::span remaining{response}; + do + { + auto poll_status = co_await client.poll(coro::poll_op::read); + REQUIRE(poll_status == coro::poll_status::event); + + auto [recv_status, r] = client.recv(remaining); + remaining = remaining.subspan(r.size_bytes()); + + } while (!remaining.empty()); + + co_return response; + }; + + auto result = coro::sync_wait(coro::when_all(make_server_task(), make_client_task())); + + auto request = std::move(std::get<0>(result).return_value()); + auto response = std::move(std::get<1>(result).return_value()); + + REQUIRE(request == response); +} + #endif // LIBCORO_FEATURE_NETWORKING diff --git a/test/net/test_tls_server.cpp b/test/net/test_tls_server.cpp index cfa94402..180472f0 100644 --- a/test/net/test_tls_server.cpp +++ b/test/net/test_tls_server.cpp @@ -7,7 +7,7 @@ #include -TEST_CASE("tls_server hello world server", "[tcp_server]") +TEST_CASE("tls_server hello world server", "[tls_server]") { auto scheduler = std::make_shared( coro::io_scheduler::options{.pool = coro::thread_pool::options{.thread_count = 1}});