From 8b7d7227881269339aabd92cd7d96b8b59a231df Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Fri, 9 Sep 2016 16:46:54 -0400 Subject: [PATCH] rgw: detect and work around a curl_multi_wait bug Fixes: http://tracker.ceph.com/issues/15915 Fixes: http://tracker.ceph.com/issues/16695 Signed-off-by: Casey Bodley (cherry picked from commit 0359be62b79857357f6d0d4fb81c6bb4e0c381a6) --- src/rgw/rgw_http_client.cc | 74 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 72 insertions(+), 2 deletions(-) diff --git a/src/rgw/rgw_http_client.cc b/src/rgw/rgw_http_client.cc index 2fa5c91140bca..1dc577d59a39e 100644 --- a/src/rgw/rgw_http_client.cc +++ b/src/rgw/rgw_http_client.cc @@ -347,7 +347,8 @@ RGWHTTPClient::~RGWHTTPClient() static int clear_signal(int fd) { // since we're in non-blocking mode, we can try to read a lot more than - // one signal from signal_thread() to avoid later wakeups + // one signal from signal_thread() to avoid later wakeups. non-blocking reads + // are also required to support the curl_multi_wait bug workaround std::array buf; int ret = ::read(fd, (void *)buf.data(), buf.size()); if (ret < 0) { @@ -359,6 +360,67 @@ static int clear_signal(int fd) #if HAVE_CURL_MULTI_WAIT +static std::once_flag detect_flag; +static bool curl_multi_wait_bug_present = false; + +static int detect_curl_multi_wait_bug(CephContext *cct, CURLM *handle, + int write_fd, int read_fd) +{ + int ret = 0; + + // write to write_fd so that read_fd becomes readable + uint32_t buf = 0; + ret = ::write(write_fd, &buf, sizeof(buf)); + if (ret < 0) { + ret = -errno; + ldout(cct, 0) << "ERROR: " << __func__ << "(): write() returned " << ret << dendl; + return ret; + } + + // pass read_fd in extra_fds for curl_multi_wait() + int num_fds; + struct curl_waitfd wait_fd; + + wait_fd.fd = read_fd; + wait_fd.events = CURL_WAIT_POLLIN; + wait_fd.revents = 0; + + ret = curl_multi_wait(handle, &wait_fd, 1, 0, &num_fds); + if (ret != CURLM_OK) { + ldout(cct, 0) << "ERROR: curl_multi_wait() returned " << ret << dendl; + return -EIO; + } + + // curl_multi_wait should flag revents when extra_fd is readable. if it + // doesn't, the bug is present and we can't rely on revents + if (wait_fd.revents == 0) { + curl_multi_wait_bug_present = true; + ldout(cct, 0) << "WARNING: detected a version of libcurl which contains a " + "bug in curl_multi_wait(). enabling a workaround that may degrade " + "performance slightly. please upgrade to a more recent version of " + "libcurl." << dendl; + } + + return clear_signal(read_fd); +} + +static bool is_signaled(const curl_waitfd& wait_fd) +{ + if (wait_fd.fd < 0) { + // no fd to signal + return false; + } + + if (curl_multi_wait_bug_present) { + // we can't rely on revents, so we always return true if a wait_fd is given. + // this means we'll be trying a non-blocking read on this fd every time that + // curl_multi_wait() wakes up + return true; + } + + return wait_fd.revents > 0; +} + static int do_curl_wait(CephContext *cct, CURLM *handle, int signal_fd) { int num_fds; @@ -374,7 +436,7 @@ static int do_curl_wait(CephContext *cct, CURLM *handle, int signal_fd) return -EIO; } - if (wait_fd.revents > 0) { + if (is_signaled(wait_fd)) { ret = clear_signal(signal_fd); if (ret < 0) { ldout(cct, 0) << "ERROR: " << __func__ << "(): read() returned " << ret << dendl; @@ -736,6 +798,14 @@ int RGWHTTPManager::set_threaded() return r; } +#ifdef HAVE_CURL_MULTI_WAIT + // on first initialization, use this pipe to detect whether we're using a + // buggy version of libcurl + std::call_once(detect_flag, detect_curl_multi_wait_bug, cct, + static_cast(multi_handle), + thread_pipe[1], thread_pipe[0]); +#endif + is_threaded = true; reqs_thread = new ReqsThread(this); reqs_thread->create("http_manager"); -- 2.39.5