mem leak fix (#3794)

* mem leak fix

* PR comments fix

* PR comments

* seems to work?

* clang

* curl again
This commit is contained in:
George Arama 2022-07-08 14:04:55 -07:00 committed by GitHub
parent 9ec257d9dd
commit 46eaa38dde
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 33 additions and 15 deletions

View File

@ -1300,7 +1300,9 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
// Creating a new connection is thread safe. No need to lock mutex here.
// No available connection for the pool for the required host. Create one
Log::Write(Logger::Level::Verbose, LogMsgPrefix + "Spawn new connection.");
CURL* newHandle = curl_easy_init();
auto newHandle = std::unique_ptr<CURL, CURL_deleter>(curl_easy_init());
if (!newHandle)
{
throw Azure::Core::Http::TransportException(
@ -1310,21 +1312,22 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
CURLcode result;
// Libcurl setup before open connection (url, connect_only, timeout)
if (!SetLibcurlOption(newHandle, CURLOPT_URL, request.GetUrl().GetAbsoluteUrl().data(), &result))
if (!SetLibcurlOption(
newHandle.get(), CURLOPT_URL, request.GetUrl().GetAbsoluteUrl().data(), &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string(curl_easy_strerror(result)));
}
if (port != 0 && !SetLibcurlOption(newHandle, CURLOPT_PORT, port, &result))
if (port != 0 && !SetLibcurlOption(newHandle.get(), CURLOPT_PORT, port, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
+ std::string(curl_easy_strerror(result)));
}
if (!SetLibcurlOption(newHandle, CURLOPT_CONNECT_ONLY, 1L, &result))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_CONNECT_ONLY, 1L, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
@ -1334,7 +1337,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
// Set timeout to 24h. Libcurl will fail uploading on windows if timeout is:
// timeout >= 25 days. Fails as soon as trying to upload any data
// 25 days < timeout > 1 days. Fail on huge uploads ( > 1GB)
if (!SetLibcurlOption(newHandle, CURLOPT_TIMEOUT, 60L * 60L * 24L, &result))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_TIMEOUT, 60L * 60L * 24L, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName + ". "
@ -1343,7 +1346,8 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
if (options.ConnectionTimeout != Azure::Core::Http::_detail::DefaultConnectionTimeout)
{
if (!SetLibcurlOption(newHandle, CURLOPT_CONNECTTIMEOUT_MS, options.ConnectionTimeout, &result))
if (!SetLibcurlOption(
newHandle.get(), CURLOPT_CONNECTTIMEOUT_MS, options.ConnectionTimeout, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
@ -1358,7 +1362,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
*/
if (options.Proxy)
{
if (!SetLibcurlOption(newHandle, CURLOPT_PROXY, options.Proxy->c_str(), &result))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_PROXY, options.Proxy->c_str(), &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
@ -1369,7 +1373,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
if (!options.CAInfo.empty())
{
if (!SetLibcurlOption(newHandle, CURLOPT_CAINFO, options.CAInfo.c_str(), &result))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_CAINFO, options.CAInfo.c_str(), &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
@ -1384,7 +1388,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
sslOption |= CURLSSLOPT_NO_REVOKE;
}
if (!SetLibcurlOption(newHandle, CURLOPT_SSL_OPTIONS, sslOption, &result))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_SSL_OPTIONS, sslOption, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
@ -1394,7 +1398,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
if (!options.SslVerifyPeer)
{
if (!SetLibcurlOption(newHandle, CURLOPT_SSL_VERIFYPEER, 0L, &result))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_SSL_VERIFYPEER, 0L, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
@ -1404,7 +1408,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
if (options.NoSignal)
{
if (!SetLibcurlOption(newHandle, CURLOPT_NOSIGNAL, 1L, &result))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_NOSIGNAL, 1L, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
@ -1416,7 +1420,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
// curl-transport adapter supports only HTTP/1.1
// https://github.com/Azure/azure-sdk-for-cpp/issues/2848
// 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))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_HTTP_VERSION, CURL_HTTP_VERSION_1_1, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
@ -1424,14 +1428,14 @@ 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))
if (!SetLibcurlOption(newHandle.get(), CURLOPT_SSLVERSION, CURL_SSLVERSION_TLSv1_2, &result))
{
throw Azure::Core::Http::TransportException(
_detail::DefaultFailedToGetNewConnectionTemplate + hostDisplayName
+ ". Failed enforcing TLS v1.2 or greater. " + std::string(curl_easy_strerror(result)));
}
auto performResult = curl_easy_perform(newHandle);
auto performResult = curl_easy_perform(newHandle.get());
if (performResult != CURLE_OK)
{
throw Http::TransportException(
@ -1439,7 +1443,7 @@ std::unique_ptr<CurlNetworkConnection> CurlConnectionPool::ExtractOrCreateCurlCo
+ std::string(curl_easy_strerror(performResult)));
}
return std::make_unique<CurlConnection>(newHandle, connectionKey);
return std::make_unique<CurlConnection>(newHandle.release(), connectionKey);
}
// Move the connection back to the connection pool. Push it to the front so it becomes the

View File

@ -126,4 +126,18 @@ namespace Azure { namespace Core { namespace Http { namespace _detail {
std::thread m_cleanThread;
};
/**
* @brief std::default_delete for the CURL * type , used for std::unique_ptr
*
*/
class CURL_deleter {
public:
void operator()(CURL* handle) noexcept
{
if (handle != nullptr)
{
curl_easy_cleanup(handle);
}
}
};
}}}} // namespace Azure::Core::Http::_detail