Skip to content

mem leak fix #3794

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
gearama merged 8 commits into from
Jul 8, 2022
Merged

mem leak fix #3794

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
748634d
mem leak fix
gearama Jul 5, 2022
63edf04
PR comments fix
gearama Jul 5, 2022
4c00365
PR comments
gearama Jul 8, 2022
24c1dfa
Merge branch 'Azure:main' into StressTest
gearama Jul 8, 2022
2ce0a93
seems to work?
gearama Jul 8, 2022
fcb0973
Merge branch 'StressTest' of https://github.com/gearama/azure-sdk-for…
gearama Jul 8, 2022
9a61214
clang
gearama Jul 8, 2022
4a62072
curl again
gearama Jul 8, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 23 additions & 1 deletion sdk/core/azure-core/src/http/curl/curl.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1309,23 +1309,30 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
}
CURLcode result;

// takes ownership of, and will free the new handle once it is going out of scope
Comment thread
gearama marked this conversation as resolved.
Outdated
// especially for error cases from the checks below
// auto curlConnection = std::make_unique<CurlConnection>(newHandle, connectionKey);
Comment thread
gearama marked this conversation as resolved.
Outdated

// Libcurl setup before open connection (url, connect_only, timeout)
if (!SetLibcurlOption(newHandle, CURLOPT_URL, request.GetUrl().GetAbsoluteUrl().data(), &result))
{
curl_easy_cleanup(newHandle);
Comment thread
gearama marked this conversation as resolved.
Outdated
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string(curl_easy_strerror(result)));
}

if (port != 0 && !SetLibcurlOption(newHandle, CURLOPT_PORT, port, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string(curl_easy_strerror(result)));
}

if (!SetLibcurlOption(newHandle, CURLOPT_CONNECT_ONLY, 1L, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string(curl_easy_strerror(result)));
Expand All @@ -1336,15 +1343,22 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
// 25 days < timeout > 1 days. Fail on huge uploads ( > 1GB)
if (!SetLibcurlOption(newHandle, CURLOPT_TIMEOUT, 60L * 60L * 24L, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string(curl_easy_strerror(result)));
}

if (options.ConnectionTimeout != Azure::Core::Http::_detail::DefaultConnectionTimeout)
{
if (!SetLibcurlOption(newHandle, CURLOPT_CONNECTTIMEOUT_MS, options.ConnectionTimeout, &result))
if (!SetLibcurlOption(
newHandle,

Comment thread
gearama marked this conversation as resolved.
Outdated
CURLOPT_CONNECTTIMEOUT_MS,
options.ConnectionTimeout,
&result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Fail setting connect timeout to: "
Expand All @@ -1360,6 +1374,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
{
if (!SetLibcurlOption(newHandle, CURLOPT_PROXY, options.Proxy->c_str(), &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed to set proxy to:" + options.Proxy.Value() + ". "
Expand All @@ -1371,6 +1386,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
{
if (!SetLibcurlOption(newHandle, CURLOPT_CAINFO, options.CAInfo.c_str(), &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed to set CA cert to:" + options.CAInfo + ". "
Expand All @@ -1386,6 +1402,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo

if (!SetLibcurlOption(newHandle, CURLOPT_SSL_OPTIONS, sslOption, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed to set ssl options to long bitmask:" + std::to_string(sslOption) + ". "
Expand All @@ -1396,6 +1413,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
{
if (!SetLibcurlOption(newHandle, CURLOPT_SSL_VERIFYPEER, 0L, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed to disable ssl verify peer. " + std::string(curl_easy_strerror(result)));
Expand All @@ -1406,6 +1424,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
{
if (!SetLibcurlOption(newHandle, CURLOPT_NOSIGNAL, 1L, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed to set NOSIGNAL option for libcurl. "
Expand All @@ -1418,6 +1437,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
// The libcurl uses HTTP/2 by default, if it can be negotiated with a server on handshake.
if (!SetLibcurlOption(newHandle, CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed to set libcurl HTTP/1.1" + ". " + std::string(curl_easy_strerror(result)));
Expand All @@ -1426,6 +1446,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
// Make libcurl to support only TLS v1.2 or later
if (!SetLibcurlOption(newHandle, CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1_2, &result))
{
curl_easy_cleanup(newHandle);
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed enforcing TLS v1.2 or greater. " + std::string(curl_easy_strerror(result)));
Expand All @@ -1434,6 +1455,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
auto performResult = curl_easy_perform(newHandle);
if (performResult != CURLE_OK)
{
curl_easy_cleanup(newHandle);
throw Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string(curl_easy_strerror(performResult)));
Expand Down
6 changes: 6 additions & 0 deletions sdk/core/azure-core/src/http/curl/curl_connection_private.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,12 @@ namespace Azure { namespace Core { namespace Http {
}
}

/**
* @brief Gets the connection handle
*
*/
CURL* GetHandle() { return m_handle; }

Comment thread
gearama marked this conversation as resolved.
Outdated
/**
* @brief Destructor.
* @details Cleans up CURL (invokes `curl_easy_cleanup()`).
Expand Down
⚡