github/azure-sdk-for-cpp
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Winhttp flaky test (#5356)

Browse Source 
* mroe quotes

* dssf

* attempt 2

* updated test and logs

* clang

* update PR

* changelog update

* Update sdk/core/azure-core/CHANGELOG.md

Co-authored-by: Larry Osterman <LarryOsterman@users.noreply.github.com>

* Update sdk/core/azure-core/CHANGELOG.md

Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com>

---------

Co-authored-by: Larry Osterman <LarryOsterman@users.noreply.github.com>
Co-authored-by: Anton Kolesnyk <41349689+antkmsft@users.noreply.github.com>
This commit is contained in:
George Arama 2024-02-15 15:55:03 -08:00 committed by GitHub
parent 6297347d75
commit 73bb6c63c1
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 29 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
sdk/core/azure-core/CHANGELOG.md
View File

@ -8,6 +8,8 @@
### Bugs Fixed
- [[5244]](https://github.com/Azure/azure-sdk-for-cpp/issues/5244) WinHTTP transport not closing correctly in case of a request timeout.
### Other Changes
- Move the connection back to the connection pool when HTTP error 404 was received. This may improve the performance of a multithreaded application when libcurl transport adapter is being used. (A community contribution, courtesy of _[mchelnokov](https://github.com/mchelnokov)_)

2
sdk/core/azure-core/src/http/winhttp/win_http_request.hpp
View File

@ -185,7 +185,7 @@ namespace Azure { namespace Core { namespace Http { namespace _detail {
WinHttpTransportOptions const& options);
~WinHttpRequest();
void MarkRequestHandleClosed() { m_requestHandleClosed = true; };
void Upload(Azure::Core::Http::Request& request, Azure::Core::Context const& context);
void SendRequest(Azure::Core::Http::Request& request, Azure::Core::Context const& context);
void ReceiveResponse(Azure::Core::Context const& context);

26
sdk/core/azure-core/src/http/winhttp/win_http_transport.cpp
View File

@ -529,13 +529,21 @@ namespace Azure { namespace Core { namespace Http { namespace _detail {
}
void WinHttpAction::CompleteActionWithError(DWORD_PTR stowedErrorInformation, DWORD stowedError)
{
// Note that the order of scope_exit and lock is important - this ensures that scope_exit is
// destroyed *after* lock is destroyed, ensuring that the event is not set to the signalled
// state before the lock is released.
auto scope_exit{m_actionCompleteEvent.SetEvent_scope_exit()};
std::unique_lock<std::mutex> lock(m_actionCompleteMutex);
m_stowedErrorInformation = stowedErrorInformation;
m_stowedError = stowedError;
if (m_expectedStatus != WINHTTP_CALLBACK_STATUS_HANDLE_CLOSING)
{
// Note that the order of scope_exit and lock is important - this ensures that scope_exit is
// destroyed *after* lock is destroyed, ensuring that the event is not set to the signalled
// state before the lock is released.
auto scope_exit{m_actionCompleteEvent.SetEvent_scope_exit()};
std::unique_lock<std::mutex> lock(m_actionCompleteMutex);
m_stowedErrorInformation = stowedErrorInformation;
m_stowedError = stowedError;
}
else
{
Log::Write(
Logger::Level::Verbose, "Received error while closing: " + std::to_string(stowedError));
}
}
DWORD WinHttpAction::GetStowedError()
@ -571,10 +579,9 @@ namespace Azure { namespace Core { namespace Http { namespace _detail {
{
return;
}
WinHttpAction* httpAction = reinterpret_cast<WinHttpAction*>(dwContext);
try
{
WinHttpAction* httpAction = reinterpret_cast<WinHttpAction*>(dwContext);
httpAction->OnHttpStatusOperation(
hInternet, internetStatus, statusInformation, statusInformationLength);
}
@ -585,6 +592,7 @@ namespace Azure { namespace Core { namespace Http { namespace _detail {
Logger::Level::Error,
"Request Failed Exception Thrown: " + std::string(rfe.what()) + rfe.Message);
WinHttpCloseHandle(hInternet);
httpAction->m_httpRequest->MarkRequestHandleClosed();
}
catch (std::exception const& ex)
{

15
sdk/core/azure-core/test/ut/transport_adapter_base_test.cpp
View File

@ -383,14 +383,17 @@ namespace Azure { namespace Core { namespace Test {
TEST_P(TransportAdapter, cancelRequest)
{
Azure::Core::Url hostPath(AzureSdkHttpbinServer::Delay() + "/10"); // 10 seconds delay on server
Azure::Core::Context cancelThis = Azure::Core::Context::ApplicationContext.WithDeadline(
std::chrono::system_clock::now() + std::chrono::seconds(3));
Azure::Core::Url hostPath(AzureSdkHttpbinServer::Delay() + "/2"); // 2 seconds delay on server
for (int i = 0; i < 10; i++)
{
Azure::Core::Context cancelThis = Azure::Core::Context::ApplicationContext.WithDeadline(
std::chrono::system_clock::now() + std::chrono::milliseconds(500));
auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, hostPath);
auto request = Azure::Core::Http::Request(Azure::Core::Http::HttpMethod::Get, hostPath);
// Request will be cancelled 3 seconds after sending the request.
EXPECT_THROW(m_pipeline->Send(request, cancelThis), Azure::Core::OperationCancelledException);
// Request will be canceled 500ms after sending the request.
EXPECT_THROW(m_pipeline->Send(request, cancelThis), Azure::Core::OperationCancelledException);
}
}
TEST_P(TransportAdapter, cancelTransferDownload)