Skip to content
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.

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

[RFC] rgw: Libcurl+NSS memory leak mitigation when performing keystone SSL auth #20924

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
13 changes: 13 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -448,6 +448,19 @@ if (WITH_RADOSGW)
else()
message(WARNING "ssl not found: rgw civetweb may fail to dlopen libssl libcrypto")
endif() # OPENSSL_FOUND

# curl-nss memory leak mitigation
execute_process(
COMMAND "curl-config --configure | grep \"\\-\\-with\\-nss\""
RESULT_VARIABLE RES
)
if (RES EQUAL 0)
message(STATUS "libcurl is configured with NSS as TLS backend")
set(LIBCURL_CONFIG_WITH_NSS ON)
else()
message(STATUS "libcurl is not configured with NSS as TLS backend")
set(LIBCURL_CONFIG_WITH_NSS OFF)
endif()
endif (WITH_RADOSGW)

#option for CephFS
Expand Down
13 changes: 13 additions & 0 deletions src/common/options.cc
Original file line number Diff line number Diff line change
Expand Up @@ -6080,6 +6080,19 @@ std::vector<Option> get_rgw_options() {
"of RGW instances under heavy use. If you would like "
"to turn off cache expiry, set this value to zero."),

// curl-nss memory leak mitigation
#if defined(LIBCURL_CONFIG_WITH_NSS)
Option("rgw_curl_https_ops_per_global_cleanup", Option::TYPE_UINT, Option::LEVEL_ADVANCED)
.set_default(0)
.add_service("rgw")
.set_description("Per how many curl operations to free libcurl memory.")
.set_long_description("Configures per how many https curl_easy_perform operation "
"to free the Curl/Nss accumulated memory in PK11_CreateGenericObject. "
"Applicable to keystone users authority authentication using ssl "
"and libcurl configured with NSS (--with-nss) as TLS backend "
"(in ceph.conf: rgw_s3_auth_use_keystone=true & rgw_keystone_verify_ssl=true). "
"Warning - Only single zone configuration is supported."),
#endif
});
}

Expand Down
3 changes: 3 additions & 0 deletions src/include/config-h.in.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,9 @@
/* Define if using NSS. */
#cmakedefine USE_NSS

/* Define if libcurl was configured with --with-nss. */
#cmakedefine LIBCURL_CONFIG_WITH_NSS

/* Accelio conditional compilation */
#cmakedefine HAVE_XIO

Expand Down
73 changes: 72 additions & 1 deletion src/rgw/rgw_http_client.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "rgw_http_client.h"
#include "rgw_http_errors.h"
#include "common/RefCountedObj.h"
#include "rgw_http_client_curl.h"

#include "rgw_coroutine.h"

Expand Down Expand Up @@ -368,6 +369,51 @@ static bool is_upload_request(const char *method)
return strcmp(method, "POST") == 0 || strcmp(method, "PUT") == 0;
}

// curl-nss memory leak mitigation
#if defined(LIBCURL_CONFIG_WITH_NSS)
std::mutex RGWHTTPClient::libcurl_global_cleanup_lock;
int RGWHTTPClient::curleasy_performs_count(0);
int RGWHTTPClient::curleasy_in_progress_count(0);
std::mutex RGWHTTPClient::curleasy_in_progress_lock;
std::condition_variable RGWHTTPClient::curleasy_in_progress_cond;

void RGWHTTPClient::libcurl_global_cleanup(const char *url)
{
int curleasy_performs_per_cleanup = cct->_conf->get_val<uint64_t>("rgw_curl_https_ops_per_global_cleanup");
if(curleasy_performs_per_cleanup > 0) {
bool is_ssl_url = boost::algorithm::starts_with(url, "https://");
std::lock_guard<std::mutex> cl(libcurl_global_cleanup_lock);
if(is_ssl_url && ++curleasy_performs_count > curleasy_performs_per_cleanup) {
cv_status cond_ret = cv_status::no_timeout;
if(curleasy_in_progress_count > 0) {
std::unique_lock<std::mutex> ipl(curleasy_in_progress_lock);
cond_ret = curleasy_in_progress_cond.wait_for(ipl, std::chrono::seconds(1));
}
if(cond_ret == cv_status::no_timeout) {
dout(20) << "performing curl global cleanup" << dendl;
rgw::curl::cleanup_curl();
rgw::curl::setup_curl(boost::none);
} else {
dout(0) << "WARNING: curl_libcurl_global_cleanup not performed, timeout while waiting for pending curl operations to complete" << dendl;
}
curleasy_performs_count = 0;
}
++curleasy_in_progress_count;
}
}

void RGWHTTPClient::libcurl_global_cleanup_signal_reqs_complete()
{
int curleasy_performs_per_cleanup = cct->_conf->get_val<uint64_t>("rgw_curl_https_ops_per_global_cleanup");
if(curleasy_performs_per_cleanup > 0) {
std::lock_guard<std::mutex> ipl(curleasy_in_progress_lock);
--curleasy_in_progress_count;
if(curleasy_in_progress_count == 0 && curleasy_performs_count > curleasy_performs_per_cleanup) {
curleasy_in_progress_cond.notify_one();
}
}
}
#endif
/*
* process a single simple one off request, not going through RGWHTTPManager. Not using
* req_data.
Expand All @@ -382,6 +428,11 @@ int RGWHTTPClient::process(const char *method, const char *url)
last_method = (method ? method : "");
last_url = (url ? url : "");

// curl-nss memory leak mitigation
#if defined(LIBCURL_CONFIG_WITH_NSS)
libcurl_global_cleanup(url);
#endif

auto ca = handles->get_curl_handle();
curl_handle = **ca;

Expand All @@ -407,7 +458,7 @@ int RGWHTTPClient::process(const char *method, const char *url)
curl_easy_setopt(curl_handle, CURLOPT_UPLOAD, 1L);
}
if (has_send_len) {
curl_easy_setopt(curl_handle, CURLOPT_INFILESIZE, (void *)send_len);
curl_easy_setopt(curl_handle, CURLOPT_INFILESIZE, (void *)send_len);
}
if (!verify_ssl) {
curl_easy_setopt(curl_handle, CURLOPT_SSL_VERIFYPEER, 0L);
Expand All @@ -424,6 +475,11 @@ int RGWHTTPClient::process(const char *method, const char *url)
handles->release_curl_handle(ca);
curl_slist_free_all(h);

// curl-nss memory leak mitigation
#if defined(LIBCURL_CONFIG_WITH_NSS)
libcurl_global_cleanup_signal_reqs_complete();
#endif

return ret;
}

Expand Down Expand Up @@ -939,6 +995,13 @@ int RGWHTTPManager::remove_request(RGWHTTPClient *client)
int RGWHTTPManager::process_requests(bool wait_for_data, bool *done)
{
assert(!is_threaded);
#if defined(LIBCURL_CONFIG_WITH_NSS)
int curleasy_performs_per_cleanup = cct->_conf->get_val<uint64_t>("rgw_curl_https_ops_per_global_cleanup");
if (curleasy_performs_per_cleanup > 0) {
dout(0) << __func__ << ": WARNING: curl_multi operations not supported in conjustion with rgw_curl_https_ops_per_global_cleanup" << dendl;
return -EPERM;
}
#endif

int still_running;
int mstatus;
Expand Down Expand Up @@ -1074,6 +1137,14 @@ void *RGWHTTPManager::reqs_thread_entry()

ldout(cct, 20) << __func__ << ": start" << dendl;

#if defined(LIBCURL_CONFIG_WITH_NSS)
int curleasy_performs_per_cleanup = cct->_conf->get_val<uint64_t>("rgw_curl_https_ops_per_global_cleanup");
if (curleasy_performs_per_cleanup > 0) {
dout(0) << __func__ << ": WARNING: curl_multi operations not supported in conjustion with rgw_curl_https_ops_per_global_cleanup" << dendl;
return NULL;
}
#endif

while (!going_down) {
int ret = do_curl_wait(cct, (CURLM *)multi_handle, thread_pipe[0]);
if (ret < 0) {
Expand Down
12 changes: 12 additions & 0 deletions src/rgw/rgw_http_client.h
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,18 @@ class RGWHTTPClient

std::atomic<unsigned> stopped { 0 };

// curl-nss memory leak mitigation
#if defined(LIBCURL_CONFIG_WITH_NSS)
static std::mutex libcurl_global_cleanup_lock;
static int curleasy_performs_count;
static int curleasy_in_progress_count;
static std::mutex curleasy_in_progress_lock;
static std::condition_variable curleasy_in_progress_cond;

void libcurl_global_cleanup(const char *url);
void libcurl_global_cleanup_signal_reqs_complete();
#endif

protected:
CephContext *cct;
param_vec_t headers;
Expand Down