ObjectCacher: fix off-by-one error in split

This error left a completion that should have been attached
to the right BufferHead on the left BufferHead, which would
result in the completion never being called unless the buffers
were merged before it's original read completed. This would cause
a hang in any higher level waiting for a read to complete.

The existing loop went backwards (using a forward iterator),
but stopped when the iterator reached the beginning of the map,
or when a waiter belonged to the left BufferHead.

If the first list of waiters should have been moved to the right
BufferHead, it was skipped because at that point the iterator
was at the beginning of the map, which was the main condition
of the loop.

Restructure the waiters-moving loop to go forward in the map instead,
so it's harder to make an off-by-one error.

Possibly-fixes: #3286
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit 2e862f4d183d8b57b43b0777737886f18f68bf00)

diff --git a/src/osdc/ObjectCacher.cc b/src/osdc/ObjectCacher.cc
index a46ffb3bece510adde4937097a1b66eea91ff118..d8b51089f3d65b17d20e3c512696591849639c0b 100644 (file)
--- a/src/osdc/ObjectCacher.cc
+++ b/src/osdc/ObjectCacher.cc
@@ -52,21 +52,22 @@ ObjectCacher::BufferHead *ObjectCacher::Object::split(BufferHead *left, loff_t o
     right->bl.substr_of(bl, left->length(), right->length());
     left->bl.substr_of(bl, 0, left->length());
   }
-  
+
   // move read waiters
   if (!left->waitfor_read.empty()) {
-    map<loff_t, list<Context*> >::iterator o, p = left->waitfor_read.end();
-    p--;
-    while (p != left->waitfor_read.begin()) {
-      if (p->first < right->start()) break;      
+    map<loff_t, list<Context*> >::iterator start_remove = left->waitfor_read.begin();
+    while (start_remove != left->waitfor_read.end() &&
+          start_remove->first < right->start())
+      ++start_remove;
+    for (map<loff_t, list<Context*> >::iterator p = start_remove;
+        p != left->waitfor_read.end(); ++p) {
       ldout(oc->cct, 0) << "split  moving waiters at byte " << p->first << " to right bh" << dendl;
       right->waitfor_read[p->first].swap( p->second );
-      o = p;
-      p--;
-      left->waitfor_read.erase(o);
+      assert(p->second.empty());
     }
+    left->waitfor_read.erase(start_remove, left->waitfor_read.end());
   }
-  
+
   ldout(oc->cct, 20) << "split    left is " << *left << dendl;
   ldout(oc->cct, 20) << "split   right is " << *right << dendl;
   return right;