osdc/Objecter: do watch/notify op completions synchronously

Normally we do Op completions under completion_lock but not the
session lock.  This is problematic because the register op modifies
LingerOp state that is no longer protected by s->lock against
races from, say, _linger_cancel, or a new thread sending a fresh
register.  See #10765 for a crash that is possibly caused by this.

Fix this by making a synchronous commit hook and using that while
still holding s->lock.

Signed-off-by: Sage Weil <sage@redhat.com>

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index 61098497d9795a2d2f51946867d45765a2000367..e11dcf68da55a86094aa43170938aace4a075e49 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -417,7 +417,6 @@ void Objecter::_send_linger(LingerOp *info)
   RWLock::Context lc(rwlock, RWLock::Context::TakenForWrite);
 
   vector<OSDOp> opv;
-  Context *onack = NULL;
   Context *oncommit = NULL;
   if (info->registered && info->is_watch) {
     ldout(cct, 15) << "send_linger " << info->linger_id << " reconnect" << dendl;
@@ -434,8 +433,9 @@ void Objecter::_send_linger(LingerOp *info)
   }
   Op *o = new Op(info->target.base_oid, info->target.base_oloc,
                 opv, info->target.flags | CEPH_OSD_FLAG_READ,
-                onack, oncommit,
+                NULL, NULL,
                 info->pobjver);
+  o->oncommit_sync = oncommit;
   o->snapid = info->snap;
   o->snapc = info->snapc;
   o->mtime = info->mtime;
@@ -550,7 +550,8 @@ void Objecter::_send_linger_ping(LingerOp *info)
   C_Linger_Ping *onack = new C_Linger_Ping(this, info);
   Op *o = new Op(info->target.base_oid, info->target.base_oloc,
                 opv, info->target.flags | CEPH_OSD_FLAG_READ,
-                onack, NULL, NULL);
+                NULL, NULL, NULL);
+  o->oncommit_sync = onack;
   o->target = info->target;
   o->should_resend = false;
   _send_op_account(o);
@@ -2731,7 +2732,7 @@ MOSDOp *Objecter::_prepare_osd_op(Op *op)
 
   int flags = op->target.flags;
   flags |= CEPH_OSD_FLAG_KNOWN_REDIR;
-  if (op->oncommit)
+  if (op->oncommit || op->oncommit_sync)
     flags |= CEPH_OSD_FLAG_ONDISK;
   if (op->onack)
     flags |= CEPH_OSD_FLAG_ACK;
@@ -3028,6 +3029,10 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m)
     num_uncommitted.dec();
     logger->inc(l_osdc_op_commit);
   }
+  if (op->oncommit_sync) {
+    op->oncommit_sync->complete(rc);
+    op->oncommit_sync = NULL;
+  }
 
   /* get it before we call _finish_op() */
   Mutex *completion_lock = (op->target.base_oid.name.size() ? s->get_lock(op->target.base_oid) : NULL);

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index ed44be004257fd7b20e416aebda062405e4bc327..676a2e93bf9b137ceeaea1ae548ff04913d8e039 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -1160,6 +1160,7 @@ public:
 
     int priority;
     Context *onack, *oncommit, *ontimeout;
+    Context *oncommit_sync;         // used internally by watch/notify
 
     ceph_tid_t tid;
     eversion_t replay_version;        // for op replay
@@ -1191,10 +1192,15 @@ public:
       con(NULL),
       snapid(CEPH_NOSNAP),
       outbl(NULL),
-      priority(0), onack(ac), oncommit(co),
+      priority(0),
+      onack(ac),
+      oncommit(co),
       ontimeout(NULL),
-      tid(0), attempts(0),
-      objver(ov), reply_epoch(NULL),
+      oncommit_sync(NULL),
+      tid(0),
+      attempts(0),
+      objver(ov),
+      reply_epoch(NULL),
       map_dne_bound(0),
       budgeted(false),
       should_resend(true),