From e6bc0b3d86518c5eb1020abef0560e1362610570 Mon Sep 17 00:00:00 2001 From: Billy Robert O'Neal III Date: Thu, 18 Oct 2018 13:45:08 -0700 Subject: [PATCH] Deal with even more flaky tests! --- Release/include/cpprest/http_msg.h | 3 + .../src/http/listener/http_listener_msg.cpp | 17 ++++-- .../http/client/building_request_tests.cpp | 8 +-- .../http/client/compression_tests.cpp | 2 +- .../http/client/progress_handler_tests.cpp | 56 +++++++++---------- 5 files changed, 48 insertions(+), 38 deletions(-) diff --git a/Release/include/cpprest/http_msg.h b/Release/include/cpprest/http_msg.h index 39649ef833..be41832e9d 100644 --- a/Release/include/cpprest/http_msg.h +++ b/Release/include/cpprest/http_msg.h @@ -878,6 +878,9 @@ class _http_request final : public http::details::http_msg_base, public std::ena http::method m_method; // Tracks whether or not a response has already been started for this message. + // 0 = No reply sent + // 1 = Usual reply sent + // 2 = Reply aborted by another thread; e.g. server shutdown pplx::details::atomic_long m_initiated_response; std::unique_ptr m_server_context; diff --git a/Release/src/http/listener/http_listener_msg.cpp b/Release/src/http/listener/http_listener_msg.cpp index 82c285cfbc..dfdb4e95f2 100644 --- a/Release/src/http/listener/http_listener_msg.cpp +++ b/Release/src/http/listener/http_listener_msg.cpp @@ -58,7 +58,7 @@ pplx::task details::_http_request::_reply_impl(http_response response) pplx::task details::_http_request::_reply_if_not_already(status_code status) { const long expected = 0; - const long desired = 1; + const long desired = 2; if (pplx::details::atomic_compare_exchange(m_initiated_response, desired, expected) == expected) { return _reply_impl(http_response(status)); @@ -68,11 +68,18 @@ pplx::task details::_http_request::_reply_if_not_already(status_code statu pplx::task details::_http_request::reply(const http_response &response) { - if(pplx::details::atomic_increment(m_initiated_response) != 1l) - { - throw http_exception(U("Error: trying to send multiple responses to an HTTP request")); + const long expected = 0; + const long desired = 1; + switch (pplx::details::atomic_compare_exchange(m_initiated_response, desired, expected)) { + case 0: + return _reply_impl(response); // success + case 1: + throw http_exception(U("Error: trying to send multiple responses to an HTTP request")); + case 2: + return pplx::task_from_result(); // already handled + default: + abort(); } - return _reply_impl(response); } }} // namespace web::http diff --git a/Release/tests/functional/http/client/building_request_tests.cpp b/Release/tests/functional/http/client/building_request_tests.cpp index a12ddfe8fc..72925b7152 100644 --- a/Release/tests/functional/http/client/building_request_tests.cpp +++ b/Release/tests/functional/http/client/building_request_tests.cpp @@ -113,7 +113,7 @@ TEST_FIXTURE(uri_address, body_types) p_request->reply(200); }); http_asserts::assert_response_equals(client.request(msg).get(), status_codes::OK); - + // string - no content type. msg = http_request(method); msg.set_body(std::move(str_move_body)); @@ -143,7 +143,7 @@ TEST(set_body_string_with_charset) { http_request request; VERIFY_THROWS(request.set_body( - ::utility::conversions::to_utf16string("body_data"), + ::utility::conversions::to_utf16string("body_data"), ::utility::conversions::to_utf16string("text/plain;charset=utf-16")), std::invalid_argument); } @@ -236,7 +236,7 @@ TEST_FIXTURE(uri_address, set_body_with_charset) http_request msg(methods::PUT); msg.set_body("datadatadata", "text/plain;charset=us-ascii"); VERIFY_THROWS(msg.set_body( - ::utility::conversions::to_utf16string("datadatadata"), + ::utility::conversions::to_utf16string("datadatadata"), ::utility::conversions::to_utf16string("text/plain;charset=us-ascii")), std::invalid_argument); } @@ -321,4 +321,4 @@ TEST_FIXTURE(uri_address, reuse_request) } -}}}} \ No newline at end of file +}}}} diff --git a/Release/tests/functional/http/client/compression_tests.cpp b/Release/tests/functional/http/client/compression_tests.cpp index 8823e2a448..d6d8ea1732 100644 --- a/Release/tests/functional/http/client/compression_tests.cpp +++ b/Release/tests/functional/http/client/compression_tests.cpp @@ -1108,7 +1108,7 @@ SUITE(compression_tests) } else { - memcpy(vv.data(), v.data(), v.size()); + std::copy(v.begin(), v.end(), vv.begin()); got = v.size(); } VERIFY_ARE_EQUAL(buffer_size, got); diff --git a/Release/tests/functional/http/client/progress_handler_tests.cpp b/Release/tests/functional/http/client/progress_handler_tests.cpp index 4b9568c36e..a4e1c8a968 100644 --- a/Release/tests/functional/http/client/progress_handler_tests.cpp +++ b/Release/tests/functional/http/client/progress_handler_tests.cpp @@ -37,13 +37,13 @@ TEST_FIXTURE(uri_address, set_progress_handler_no_bodies) http_request msg(mtd); msg.set_progress_handler( - [&](message_direction::direction direction, utility::size64_t so_far) - { + [&](message_direction::direction direction, utility::size64_t so_far) + { calls += 1; - if (direction == message_direction::upload) - upsize = so_far; - else - downsize = so_far; + if (direction == message_direction::upload) + upsize = so_far; + else + downsize = so_far; }); test_http_server::scoped_server scoped(m_uri); @@ -123,19 +123,19 @@ TEST_FIXTURE(uri_address, set_progress_handler_download) http_client client(m_uri, config); const method mtd = methods::GET; - + utility::size64_t upsize = 4711u, downsize = 4711u; int calls = 0; http_request msg(mtd); msg.set_progress_handler( - [&](message_direction::direction direction, utility::size64_t so_far) - { + [&](message_direction::direction direction, utility::size64_t so_far) + { calls += 1; - if (direction == message_direction::upload) - upsize = so_far; - else - downsize = so_far; + if (direction == message_direction::upload) + upsize = so_far; + else + downsize = so_far; }); const size_t repeats = 6000; @@ -179,19 +179,19 @@ TEST_FIXTURE(uri_address, set_progress_handler_upload_and_download) const size_t repeats = 5500; for (size_t i = 0; i < repeats; ++i) data.append(U("abcdefghihklmnopqrstuvwxyz")); - + utility::size64_t upsize = 4711u, downsize = 4711u; int calls = 0; http_request msg(mtd); msg.set_progress_handler( - [&](message_direction::direction direction, utility::size64_t so_far) - { + [&](message_direction::direction direction, utility::size64_t so_far) + { calls += 1; - if (direction == message_direction::upload) - upsize = so_far; - else - downsize = so_far; + if (direction == message_direction::upload) + upsize = so_far; + else + downsize = so_far; }); msg.set_body(data); @@ -274,7 +274,7 @@ TEST_FIXTURE(uri_address, set_progress_handler_request_timeout) const size_t repeats = 5500; for (size_t i = 0; i < repeats; ++i) data.append(U("abcdefghihklmnopqrstuvwxyz")); - + utility::size64_t upsize = 4711u, downsize = 4711u; int calls = 0; @@ -282,19 +282,19 @@ TEST_FIXTURE(uri_address, set_progress_handler_request_timeout) // We should never see this handler called for download, but for upload should still happen, since // there's a server (just not a very responsive one) and we're sending data to it. msg.set_progress_handler( - [&](message_direction::direction direction, utility::size64_t so_far) - { + [&](message_direction::direction direction, utility::size64_t so_far) + { calls += 1; - if (direction == message_direction::upload) - upsize = so_far; - else - downsize = so_far; + if (direction == message_direction::upload) + upsize = so_far; + else + downsize = so_far; }); msg.set_body(data); auto t = scoped.server()->next_request(); auto response = client.request(msg); - + #ifdef __APPLE__ // CodePlex 295 VERIFY_THROWS(response.get(), http_exception);