From: Jason Dillaman <dillaman@redhat.com>
Date: Fri, 10 Jul 2020 16:46:27 +0000 (-0400)
Subject: librbd: allocate the asio strands directly on the heap
X-Git-Tag: v16.1.0~1685^2~5
X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=9b6a2040b727599ef41a4f9a765a2cb1dfee1f98;p=ceph.git

librbd: allocate the asio strands directly on the heap

This will assist with potential race condition debugging since the
stand pointer will be invalidated by the time the strand has been
destructed and shut down.

Signed-off-by: Jason Dillaman <dillaman@redhat.com>
---

diff --git a/src/librbd/AsioEngine.cc b/src/librbd/AsioEngine.cc
index d8cf48d39690..1a46b590479d 100644
--- a/src/librbd/AsioEngine.cc
+++ b/src/librbd/AsioEngine.cc
@@ -21,7 +21,8 @@ AsioEngine::AsioEngine(std::shared_ptr<librados::Rados> rados)
       neorados::RADOS::make_with_librados(*rados))),
     m_cct(m_rados_api->cct()),
     m_io_context(m_rados_api->get_io_context()),
-    m_api_strand(m_io_context),
+    m_api_strand(std::make_unique<boost::asio::io_context::strand>(
+      m_io_context)),
     m_context_wq(std::make_unique<asio::ContextWQ>(m_cct, m_io_context)) {
   ldout(m_cct, 20) << dendl;
 
@@ -40,6 +41,7 @@ AsioEngine::AsioEngine(librados::IoCtx& io_ctx)
 
 AsioEngine::~AsioEngine() {
   ldout(m_cct, 20) << dendl;
+  m_api_strand.reset();
 }
 
 void AsioEngine::dispatch(Context* ctx, int r) {
diff --git a/src/librbd/AsioEngine.h b/src/librbd/AsioEngine.h
index 00cb7f7f8a10..d2730fd98804 100644
--- a/src/librbd/AsioEngine.h
+++ b/src/librbd/AsioEngine.h
@@ -45,7 +45,7 @@ public:
 
   inline boost::asio::io_context::strand& get_api_strand() {
     // API client callbacks should never fire concurrently
-    return m_api_strand;
+    return *m_api_strand;
   }
 
   inline asio::ContextWQ* get_work_queue() {
@@ -69,7 +69,7 @@ private:
   CephContext* m_cct;
 
   boost::asio::io_context& m_io_context;
-  boost::asio::io_context::strand m_api_strand;
+  std::unique_ptr<boost::asio::io_context::strand> m_api_strand;
   std::unique_ptr<asio::ContextWQ> m_context_wq;
 };
 
diff --git a/src/librbd/asio/ContextWQ.cc b/src/librbd/asio/ContextWQ.cc
index 2d12d5080fd1..4f6c72770806 100644
--- a/src/librbd/asio/ContextWQ.cc
+++ b/src/librbd/asio/ContextWQ.cc
@@ -15,7 +15,8 @@ namespace librbd {
 namespace asio {
 
 ContextWQ::ContextWQ(CephContext* cct, boost::asio::io_context& io_context)
-  : m_cct(cct), m_io_context(io_context), m_strand(io_context),
+  : m_cct(cct), m_io_context(io_context),
+    m_strand(std::make_unique<boost::asio::io_context::strand>(io_context)),
     m_queued_ops(0) {
   ldout(m_cct, 20) << dendl;
 }
@@ -23,6 +24,7 @@ ContextWQ::ContextWQ(CephContext* cct, boost::asio::io_context& io_context)
 ContextWQ::~ContextWQ() {
   ldout(m_cct, 20) << dendl;
   drain();
+  m_strand.reset();
 }
 
 void ContextWQ::drain() {
@@ -40,7 +42,7 @@ void ContextWQ::drain_handler(Context* ctx) {
 
   // new items might be queued while we are trying to drain, so we
   // might need to post the handler multiple times
-  boost::asio::post(m_strand, [this, ctx]() { drain_handler(ctx); });
+  boost::asio::post(*m_strand, [this, ctx]() { drain_handler(ctx); });
 }
 
 } // namespace asio
diff --git a/src/librbd/asio/ContextWQ.h b/src/librbd/asio/ContextWQ.h
index 3197756205d3..85c254161213 100644
--- a/src/librbd/asio/ContextWQ.h
+++ b/src/librbd/asio/ContextWQ.h
@@ -7,6 +7,7 @@
 #include "include/common_fwd.h"
 #include "include/Context.h"
 #include <atomic>
+#include <memory>
 #include <boost/asio/io_context.hpp>
 #include <boost/asio/io_context_strand.hpp>
 #include <boost/asio/post.hpp>
@@ -26,7 +27,7 @@ public:
 
     // ensure all legacy ContextWQ users are dispatched sequentially for
     // backwards compatibility (i.e. might not be concurrent thread-safe)
-    boost::asio::post(m_strand, [this, ctx, r]() {
+    boost::asio::post(*m_strand, [this, ctx, r]() {
       ctx->complete(r);
 
       ceph_assert(m_queued_ops > 0);
@@ -37,7 +38,7 @@ public:
 private:
   CephContext* m_cct;
   boost::asio::io_context& m_io_context;
-  boost::asio::io_context::strand m_strand;
+  std::unique_ptr<boost::asio::io_context::strand> m_strand;
 
   std::atomic<uint64_t> m_queued_ops;