osdc/Objecter: fix narrow race with tid assignment

The operation completion could finish and be freed before we
do the info->register_tid assignment.  Avoid this by doing the
assignment in _op_submit itself.

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

diff --git a/src/osdc/Objecter.cc b/src/osdc/Objecter.cc
index d184b446b367954de87f23e28d8d214629331587..3664585b2a4ca571c787f7242f9de724ddaf3606 100644 (file)
--- a/src/osdc/Objecter.cc
+++ b/src/osdc/Objecter.cc
@@ -542,10 +542,10 @@ void Objecter::_send_linger(LingerOp *info,
     }
     sl.unlock();
 
-    info->register_tid = _op_submit(o, sul);
+    _op_submit(o, sul, &info->register_tid);
   } else {
     // first send
-    info->register_tid = _op_submit_with_budget(o, sul);
+    _op_submit_with_budget(o, sul, &info->register_tid);
   }
 
   logger->inc(l_osdc_linger_send);
@@ -2128,11 +2128,14 @@ void Objecter::resend_mon_ops()
 ceph_tid_t Objecter::op_submit(Op *op, int *ctx_budget)
 {
   shunique_lock rl(rwlock, ceph::acquire_shared);
-  return _op_submit_with_budget(op, rl, ctx_budget);
+  ceph_tid_t tid = 0;
+  _op_submit_with_budget(op, rl, &tid, ctx_budget);
+  return tid;
 }
 
-ceph_tid_t Objecter::_op_submit_with_budget(Op *op, shunique_lock& sul,
-                                           int *ctx_budget)
+void Objecter::_op_submit_with_budget(Op *op, shunique_lock& sul,
+                                     ceph_tid_t *ptid,
+                                     int *ctx_budget)
 {
   assert(initialized.read());
 
@@ -2160,7 +2163,7 @@ ceph_tid_t Objecter::_op_submit_with_budget(Op *op, shunique_lock& sul,
                                      op_cancel(tid, -ETIMEDOUT); });
   }
 
-  return _op_submit(op, sul);
+  _op_submit(op, sul, ptid);
 }
 
 void Objecter::_send_op_account(Op *op)
@@ -2242,7 +2245,7 @@ void Objecter::_send_op_account(Op *op)
   }
 }
 
-ceph_tid_t Objecter::_op_submit(Op *op, shunique_lock& sul)
+void Objecter::_op_submit(Op *op, shunique_lock& sul, ceph_tid_t *ptid)
 {
   // rwlock is locked
 
@@ -2335,6 +2338,8 @@ ceph_tid_t Objecter::_op_submit(Op *op, shunique_lock& sul)
   if (check_for_latest_map) {
     _send_op_map_check(op);
   }
+  if (ptid)
+    *ptid = tid;
   op = NULL;
 
   sl.unlock();
@@ -2342,8 +2347,6 @@ ceph_tid_t Objecter::_op_submit(Op *op, shunique_lock& sul)
 
   ldout(cct, 5) << num_unacked.read() << " unacked, " << num_uncommitted.read()
                << " uncommitted" << dendl;
-
-  return tid;
 }
 
 int Objecter::op_cancel(OSDSession *s, ceph_tid_t tid, int r)
@@ -3211,7 +3214,7 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m)
     m->get_redirect().combine_with_locator(op->target.target_oloc,
                                           op->target.target_oid.name);
     op->target.flags |= CEPH_OSD_FLAG_REDIRECTED;
-    _op_submit(op, sul);
+    _op_submit(op, sul, NULL);
     m->put();
     return;
   }

diff --git a/src/osdc/Objecter.h b/src/osdc/Objecter.h
index 05b29bbb91ffab09b7d897744aab3016cc57e342..478f555a84c66657f0d944b4ebc145066303012f 100644 (file)
--- a/src/osdc/Objecter.h
+++ b/src/osdc/Objecter.h
@@ -2074,9 +2074,10 @@ private:
 private:
 
   // low-level
-  ceph_tid_t _op_submit(Op *op, shunique_lock& lc);
-  ceph_tid_t _op_submit_with_budget(Op *op, shunique_lock& lc,
-                                   int *ctx_budget = NULL);
+  void _op_submit(Op *op, shunique_lock& lc, ceph_tid_t *ptid);
+  void _op_submit_with_budget(Op *op, shunique_lock& lc,
+                             ceph_tid_t *ptid,
+                             int *ctx_budget = NULL);
   inline void unregister_op(Op *op);
 
   // public interface