rgw/http: use a dedicated mutex for reqs_change_state

set_request_state() pushed into reqs_change_state without holding
any lock. Concurrent callers and manage_pending_requests raced on
the list and corrupted node links, crashing in std::list::_M_hook.

Use a dedicated mutex; reusing reqs_lock would invert the
completion path's reqs_lock -> req_data->lock order against the
set_request_state callers, which already hold req_data->lock.

Signed-off-by: Matthew N. Heler <matthew.heler@hotmail.com>

diff --git a/src/rgw/rgw_http_client.cc b/src/rgw/rgw_http_client.cc
index dc798943ee49d8604de4b86000b1d2816c0f6da9..62b59d829138e31110745eb6674c3504ce1731ab 100644 (file)
--- a/src/rgw/rgw_http_client.cc
+++ b/src/rgw/rgw_http_client.cc
@@ -891,10 +891,16 @@ void RGWHTTPManager::unlink_request(rgw_http_req_data *req_data)
 
 void RGWHTTPManager::manage_pending_requests()
 {
+  std::list<set_state> changes;
+  {
+    std::lock_guard l{reqs_change_state_lock};
+    changes.swap(reqs_change_state);
+  }
+
   reqs_lock.lock_shared();
   if (max_threaded_req == num_reqs &&
       unregistered_reqs.empty() &&
-      reqs_change_state.empty()) {
+      changes.empty()) {
     reqs_lock.unlock_shared();
     return;
   }
@@ -902,11 +908,8 @@ void RGWHTTPManager::manage_pending_requests()
 
   std::unique_lock wl{reqs_lock};
 
-  if (!reqs_change_state.empty()) {
-    for (auto siter : reqs_change_state) {
-      _set_req_state(siter);
-    }
-    reqs_change_state.clear();
+  for (auto& siter : changes) {
+    _set_req_state(siter);
   }
 
   if (!unregistered_reqs.empty()) {
@@ -1046,7 +1049,10 @@ int RGWHTTPManager::set_request_state(RGWHTTPClient *client, RGWHTTPRequestSetSt
     bitmask |= CURLPAUSE_RECV;
   }
 
-  reqs_change_state.push_back(set_state(req_data, bitmask));
+  {
+    std::lock_guard l{reqs_change_state_lock};
+    reqs_change_state.push_back(set_state(req_data, bitmask));
+  }
   int ret = signal_thread();
   if (ret < 0) {
     return ret;

diff --git a/src/rgw/rgw_http_client.h b/src/rgw/rgw_http_client.h
index 37b3991297aee35dbf6efca2b306c4a9437ddac0..c2cb2632f68df848d778e29919ad46d98fbb4531 100644 (file)
--- a/src/rgw/rgw_http_client.h
+++ b/src/rgw/rgw_http_client.h
@@ -307,6 +307,8 @@ class RGWHTTPManager {
   ceph::shared_mutex reqs_lock = ceph::make_shared_mutex("RGWHTTPManager::reqs_lock");
   std::map<uint64_t, rgw_http_req_data *> reqs;
   std::list<rgw_http_req_data *> unregistered_reqs;
+  ceph::mutex reqs_change_state_lock =
+    ceph::make_mutex("RGWHTTPManager::reqs_change_state_lock");
   std::list<set_state> reqs_change_state;
   std::map<uint64_t, rgw_http_req_data *> complete_reqs;
   int64_t num_reqs = 0;