osdc/Objecter: prevent pool dne check from invalidating scan_requests iterator

We iterate over ops and, if the pool dne and other conditions are true,
we will immediately return ENOENT and cancel an op.  Increment the
iterator at the top of the loop to avoid invalidating it.

We also need to switch to a map<>, because hash_map<> mutations may
invalidate any/all iterators.

Fixes: #3613
Signed-off-by: Sage Weil <sage@inktank.com>

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 9d7fe67cf9d2e14505b2363a0170434c0d7001a1..e29d879133b427d8831b0907dde93f208f999f58 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -439,10 +439,10 @@ void Objecter::scan_requests(bool skipped_map,
   }
 
   // check for changed request mappings
-  for (hash_map<tid_t,Op*>::iterator p = ops.begin();
-       p != ops.end();
-       ++p) {
+  map<tid_t,Op*>::iterator p = ops.begin();
+  while (p != ops.end()) {
     Op *op = p->second;
+    ++p;   // check_op_pool_dne() may touch ops; prevent iterator invalidation
     ldout(cct, 10) << " checking op " << op->tid << dendl;
     int r = recalc_op_target(op);
     switch (r) {
@@ -568,7 +568,7 @@ void Objecter::handle_osd_map(MOSDMap *m)
   // unpause requests?
   if ((was_pauserd && !pauserd) ||
       (was_pausewr && !pausewr))
-    for (hash_map<tid_t,Op*>::iterator p = ops.begin();
+    for (map<tid_t,Op*>::iterator p = ops.begin();
         p != ops.end();
         p++) {
       Op *op = p->second;
@@ -900,7 +900,7 @@ void Objecter::tick()
   cutoff -= cct->_conf->objecter_timeout;  // timeout
 
   unsigned laggy_ops = 0;
-  for (hash_map<tid_t,Op*>::iterator p = ops.begin();
+  for (map<tid_t,Op*>::iterator p = ops.begin();
        p != ops.end();
        p++) {
     Op *op = p->second;
@@ -2010,7 +2010,7 @@ void Objecter::ms_handle_remote_reset(Connection *con)
 void Objecter::dump_active()
 {
   ldout(cct, 20) << "dump_active .. " << num_homeless_ops << " homeless" << dendl;
-  for (hash_map<tid_t,Op*>::iterator p = ops.begin(); p != ops.end(); p++) {
+  for (map<tid_t,Op*>::iterator p = ops.begin(); p != ops.end(); p++) {
     Op *op = p->second;
     ldout(cct, 20) << op->tid << "\t" << op->pgid << "\tosd." << (op->session ? op->session->osd : -1)
            << "\t" << op->oid << "\t" << op->ops << dendl;
@@ -2033,7 +2033,7 @@ void Objecter::dump_requests(Formatter& fmt) const
 void Objecter::dump_ops(Formatter& fmt) const
 {
   fmt.open_array_section("ops");
-  for (hash_map<tid_t,Op*>::const_iterator p = ops.begin();
+  for (map<tid_t,Op*>::const_iterator p = ops.begin();
        p != ops.end();
        ++p) {
     Op *op = p->second;

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index e23f32a8d0f0822a383386d59dacf282bb3b42f5..bba7e9944c213eb782062e05dfe96d700ed03309 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -28,7 +28,6 @@
 #include <list>
 #include <map>
 #include <memory>
-#include <ext/hash_map>
 using namespace std;
 using namespace __gnu_cxx;
 
@@ -884,7 +883,7 @@ public:
 
  private:
   // pending ops
-  hash_map<tid_t,Op*>       ops;
+  map<tid_t,Op*>            ops;
   int                       num_homeless_ops;
   map<uint64_t, LingerOp*>  linger_ops;
   map<tid_t,PoolStatOp*>    poolstat_ops;