Skip to content

Commit

Permalink
librbd: allocate the asio strands directly on the heap
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Jason Dillaman committed Jul 16, 2020
1 parent fed9f94 commit 9b6a204
Show file tree
Hide file tree
Showing 4 changed files with 12 additions and 7 deletions.
4 changes: 3 additions & 1 deletion src/librbd/AsioEngine.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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) {
Expand Down
4 changes: 2 additions & 2 deletions src/librbd/AsioEngine.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ class AsioEngine {

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() {
Expand All @@ -69,7 +69,7 @@ class AsioEngine {
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;
};

Expand Down
6 changes: 4 additions & 2 deletions src/librbd/asio/ContextWQ.cc
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,16 @@ 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;
}

ContextWQ::~ContextWQ() {
ldout(m_cct, 20) << dendl;
drain();
m_strand.reset();
}

void ContextWQ::drain() {
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/librbd/asio/ContextWQ.h
Original file line number Diff line number Diff line change
Expand Up @@ -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>
Expand All @@ -26,7 +27,7 @@ class ContextWQ {

// 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);
Expand All @@ -37,7 +38,7 @@ class ContextWQ {
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;

Expand Down

0 comments on commit 9b6a204

Please sign in to comment.