From 73d0ee1b8097a0cdbe0c695b55a4ce323dbf5f5c Mon Sep 17 00:00:00 2001 From: Casey Bodley Date: Tue, 1 Mar 2022 13:21:39 -0500 Subject: [PATCH] Revert "rgw: detect and work around a curl_multi_wait bug" This reverts commit 0359be62b79857357f6d0d4fb81c6bb4e0c381a6. Signed-off-by: Casey Bodley Conflicts: src/rgw/rgw_http_client.cc array initializer, is_threaded->is_started --- src/rgw/rgw_http_client.cc | 131 +------------------------------------ 1 file changed, 2 insertions(+), 129 deletions(-) diff --git a/src/rgw/rgw_http_client.cc b/src/rgw/rgw_http_client.cc index 0e3c79d8562..5bf977d4f3b 100644 --- a/src/rgw/rgw_http_client.cc +++ b/src/rgw/rgw_http_client.cc @@ -721,8 +721,7 @@ int RGWHTTPTransceiver::send_data(void* ptr, size_t len, bool* pause) 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. non-blocking reads - // are also required to support the curl_multi_wait bug workaround + // one signal from signal_thread() to avoid later wakeups std::array buf{}; int ret = ::read(fd, (void *)buf.data(), buf.size()); if (ret < 0) { @@ -732,68 +731,6 @@ static int clear_signal(int fd) return 0; } -#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." << 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; @@ -809,72 +746,16 @@ static int do_curl_wait(CephContext *cct, CURLM *handle, int signal_fd) return -EIO; } - if (is_signaled(wait_fd)) { - ret = clear_signal(signal_fd); - if (ret < 0) { - ldout(cct, 0) << "ERROR: " << __func__ << "(): read() returned " << ret << dendl; - return ret; - } - } - return 0; -} - -#else - -static int do_curl_wait(CephContext *cct, CURLM *handle, int signal_fd) -{ - fd_set fdread; - fd_set fdwrite; - fd_set fdexcep; - int maxfd = -1; - - FD_ZERO(&fdread); - FD_ZERO(&fdwrite); - FD_ZERO(&fdexcep); - - /* get file descriptors from the transfers */ - int ret = curl_multi_fdset(handle, &fdread, &fdwrite, &fdexcep, &maxfd); - if (ret) { - ldout(cct, 0) << "ERROR: curl_multi_fdset returned " << ret << dendl; - return -EIO; - } - - if (signal_fd > 0) { - FD_SET(signal_fd, &fdread); - if (signal_fd >= maxfd) { - maxfd = signal_fd + 1; - } - } - - /* forcing a strict timeout, as the returned fdsets might not reference all fds we wait on */ - uint64_t to = cct->_conf->rgw_curl_wait_timeout_ms; -#define RGW_CURL_TIMEOUT 1000 - if (!to) - to = RGW_CURL_TIMEOUT; - struct timeval timeout; - timeout.tv_sec = to / 1000; - timeout.tv_usec = to % 1000; - - ret = select(maxfd+1, &fdread, &fdwrite, &fdexcep, &timeout); - if (ret < 0) { - ret = -errno; - ldout(cct, 0) << "ERROR: select returned " << ret << dendl; - return ret; - } - - if (signal_fd > 0 && FD_ISSET(signal_fd, &fdread)) { + if (wait_fd.revents > 0) { ret = clear_signal(signal_fd); if (ret < 0) { ldout(cct, 0) << "ERROR: " << __func__ << "(): read() returned " << ret << dendl; return ret; } } - return 0; } -#endif - void *RGWHTTPManager::ReqsThread::entry() { manager->reqs_thread_entry(); @@ -1174,14 +1055,6 @@ int RGWHTTPManager::start() return -e; } -#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_started = true; reqs_thread = new ReqsThread(this); reqs_thread->create("http_manager"); -- 2.39.5