Skip to content

Commit 24e67fa

Browse files
adamemersonadamemerson
committed
neorados: Hold reference to implementation across operations
Asynchrony combined with cancellations keeps leading to occasional lifetime issues, so follow the best-practices of Asio I/O objects by having completions keep a reference live. The original NeoRados backing implements Asio's two-phase shutdown properly. The RadosClient backing does not, because it shares an Objecter with completions that do not belong to it. In practice I don't think this will matter since librados and neorados get shut down around the same time. Signed-off-by: Adam C. Emerson <[email protected]>
1 parent 8f6dd33 commit 24e67fa

File tree

5 files changed

+106
-116
lines changed

5 files changed

+106
-116
lines changed

‎src/include/neorados/RADOS.hpp‎

Lines changed: 45 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@
2424
#include <string>
2525
#include <string_view>
2626
#include <type_traits>
27-
#include <variant>
2827

2928
#include <fmt/format.h>
3029
#include <fmt/ostream.h>
@@ -1373,8 +1372,8 @@ class RADOS final
13731372

13741373
static RADOS make_with_librados(librados::Rados& rados);
13751374

1376-
RADOS(const RADOS&) = delete;
1377-
RADOS& operator =(const RADOS&) = delete;
1375+
RADOS(const RADOS&);
1376+
RADOS& operator =(const RADOS&);
13781377

13791378
RADOS(RADOS&&);
13801379
RADOS& operator =(RADOS&&);
@@ -1387,14 +1386,26 @@ class RADOS final
13871386
executor_type get_executor() const;
13881387
boost::asio::io_context& get_io_context();
13891388

1389+
private:
1390+
template<typename CompletionToken>
1391+
auto consign(CompletionToken&& token) {
1392+
return boost::asio::consign(
1393+
std::forward<CompletionToken>(token),
1394+
std::make_pair(
1395+
boost::asio::make_work_guard(
1396+
boost::asio::get_associated_executor(token, get_executor())),
1397+
impl));
1398+
}
1399+
1400+
public:
1401+
1402+
13901403
template<boost::asio::completion_token_for<Op::Signature> CompletionToken>
13911404
auto execute(Object o, IOContext ioc, ReadOp op,
13921405
ceph::buffer::list* bl,
13931406
CompletionToken&& token, uint64_t* objver = nullptr,
13941407
const blkin_trace_info* trace_info = nullptr) {
1395-
auto consigned = boost::asio::consign(
1396-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1397-
boost::asio::get_associated_executor(token, get_executor())));
1408+
auto consigned = consign(std::forward<CompletionToken>(token));
13981409
return boost::asio::async_initiate<decltype(consigned), Op::Signature>(
13991410
[bl, objver, trace_info, this](auto&& handler, Object o, IOContext ioc,
14001411
ReadOp op) {
@@ -1407,9 +1418,7 @@ class RADOS final
14071418
auto execute(Object o, IOContext ioc, WriteOp op,
14081419
CompletionToken&& token, uint64_t* objver = nullptr,
14091420
const blkin_trace_info* trace_info = nullptr) {
1410-
auto consigned = boost::asio::consign(
1411-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1412-
boost::asio::get_associated_executor(token, get_executor())));
1421+
auto consigned = consign(std::forward<CompletionToken>(token));
14131422
return boost::asio::async_initiate<decltype(consigned), Op::Signature>(
14141423
[objver, trace_info, this](auto&& handler, Object o, IOContext ioc,
14151424
WriteOp op) {
@@ -1425,9 +1434,7 @@ class RADOS final
14251434
using LookupPoolComp = boost::asio::any_completion_handler<LookupPoolSig>;
14261435
template<boost::asio::completion_token_for<LookupPoolSig> CompletionToken>
14271436
auto lookup_pool(std::string name, CompletionToken&& token) {
1428-
auto consigned = boost::asio::consign(
1429-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1430-
boost::asio::get_associated_executor(token, get_executor())));
1437+
auto consigned = consign(std::forward<CompletionToken>(token));
14311438
return boost::asio::async_initiate<decltype(consigned), LookupPoolSig>(
14321439
[this](auto&& handler, std::string name) {
14331440
lookup_pool_(std::move(name), std::move(handler));
@@ -1440,9 +1447,7 @@ class RADOS final
14401447
using LSPoolsComp = boost::asio::any_completion_handler<LSPoolsSig>;
14411448
template<boost::asio::completion_token_for<LSPoolsSig> CompletionToken>
14421449
auto list_pools(CompletionToken&& token) {
1443-
auto consigned = boost::asio::consign(
1444-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1445-
boost::asio::get_associated_executor(token, get_executor())));
1450+
auto consigned = consign(std::forward<CompletionToken>(token));
14461451
return boost::asio::async_initiate<decltype(consigned), LSPoolsSig>(
14471452
[this](auto&& handler) {
14481453
list_pools_(std::move(handler));
@@ -1454,9 +1459,7 @@ class RADOS final
14541459
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
14551460
auto create_pool_snap(int64_t pool, std::string snap_name,
14561461
CompletionToken&& token) {
1457-
auto consigned = boost::asio::consign(
1458-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1459-
boost::asio::get_associated_executor(token, get_executor())));
1462+
auto consigned = consign(std::forward<CompletionToken>(token));
14601463
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
14611464
[pool, this](auto&& handler, std::string snap_name) {
14621465
create_pool_snap_(pool, std::move(snap_name),
@@ -1475,9 +1478,7 @@ class RADOS final
14751478
using SMSnapComp = boost::asio::any_completion_handler<SMSnapSig>;
14761479
template<boost::asio::completion_token_for<SMSnapSig> CompletionToken>
14771480
auto allocate_selfmanaged_snap(int64_t pool, CompletionToken&& token) {
1478-
auto consigned = boost::asio::consign(
1479-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1480-
boost::asio::get_associated_executor(token, get_executor())));
1481+
auto consigned = consign(std::forward<CompletionToken>(token));
14811482
return boost::asio::async_initiate<decltype(consigned), SMSnapSig>(
14821483
[pool, this](auto&& handler) {
14831484
allocate_selfmanaged_snap_(pool, std::move(handler));
@@ -1487,9 +1488,7 @@ class RADOS final
14871488
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
14881489
auto delete_pool_snap(int64_t pool, std::string snap_name,
14891490
CompletionToken&& token) {
1490-
auto consigned = boost::asio::consign(
1491-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1492-
boost::asio::get_associated_executor(token, get_executor())));
1491+
auto consigned = consign(std::forward<CompletionToken>(token));
14931492
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
14941493
[pool, this](auto&& handler, std::string snap_name) {
14951494
delete_pool_snap_(pool, std::move(snap_name),
@@ -1500,9 +1499,7 @@ class RADOS final
15001499
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
15011500
auto delete_selfmanaged_snap(int64_t pool, std::uint64_t snap,
15021501
CompletionToken&& token) {
1503-
auto consigned = boost::asio::consign(
1504-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1505-
boost::asio::get_associated_executor(token, get_executor())));
1502+
auto consigned = consign(std::forward<CompletionToken>(token));
15061503
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
15071504
[pool, snap, this](auto&& handler) {
15081505
delete_selfmanaged_snap_(pool, snap, std::move(handler));
@@ -1545,9 +1542,7 @@ class RADOS final
15451542
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
15461543
auto create_pool(std::string name, std::optional<int> crush_rule,
15471544
CompletionToken&& token) {
1548-
auto consigned = boost::asio::consign(
1549-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1550-
boost::asio::get_associated_executor(token, get_executor())));
1545+
auto consigned = consign(std::forward<CompletionToken>(token));
15511546
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
15521547
[crush_rule, this](auto&& handler, std::string name) {
15531548
create_pool_(std::move(name), crush_rule,
@@ -1557,9 +1552,7 @@ class RADOS final
15571552

15581553
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
15591554
auto delete_pool(std::string name, CompletionToken&& token) {
1560-
auto consigned = boost::asio::consign(
1561-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1562-
boost::asio::get_associated_executor(token, get_executor())));
1555+
auto consigned = consign(std::forward<CompletionToken>(token));
15631556
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
15641557
[this](auto&& handler, std::string name) {
15651558
delete_pool_(std::move(name), std::move(handler));
@@ -1568,9 +1561,7 @@ class RADOS final
15681561

15691562
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
15701563
auto delete_pool(int64_t pool, CompletionToken&& token) {
1571-
auto consigned = boost::asio::consign(
1572-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1573-
boost::asio::get_associated_executor(token, get_executor())));
1564+
auto consigned = consign(std::forward<CompletionToken>(token));
15741565
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
15751566
[pool, this](auto&& handler) {
15761567
delete_pool_(pool, std::move(handler));
@@ -1583,9 +1574,7 @@ class RADOS final
15831574
using PoolStatComp = boost::asio::any_completion_handler<PoolStatSig>;
15841575
template<boost::asio::completion_token_for<PoolStatSig> CompletionToken>
15851576
auto stat_pools(std::vector<std::string> pools, CompletionToken&& token) {
1586-
auto consigned = boost::asio::consign(
1587-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1588-
boost::asio::get_associated_executor(token, get_executor())));
1577+
auto consigned = consign(std::forward<CompletionToken>(token));
15891578
return boost::asio::async_initiate<decltype(consigned), PoolStatSig>(
15901579
[this](auto&& handler, std::vector<std::string> pools) {
15911580
stat_pools_(std::move(pools), std::move(handler));
@@ -1597,9 +1586,7 @@ class RADOS final
15971586
using StatFSComp = boost::asio::any_completion_handler<StatFSSig>;
15981587
template<boost::asio::completion_token_for<StatFSSig> CompletionToken>
15991588
auto statfs(std::optional<int64_t> pool, CompletionToken&& token) {
1600-
auto consigned = boost::asio::consign(
1601-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1602-
boost::asio::get_associated_executor(token, get_executor())));
1589+
auto consigned = consign(std::forward<CompletionToken>(token));
16031590
return boost::asio::async_initiate<decltype(consigned), StatFSSig>(
16041591
[pool, this](auto&& handler) {
16051592
statfs_(pool, std::move(handler));
@@ -1619,9 +1606,7 @@ class RADOS final
16191606
auto watch(Object o, IOContext ioc,
16201607
std::optional<std::chrono::seconds> timeout,
16211608
WatchCB cb, CompletionToken&& token) {
1622-
auto consigned = boost::asio::consign(
1623-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1624-
boost::asio::get_associated_executor(token, get_executor())));
1609+
auto consigned = consign(std::forward<CompletionToken>(token));
16251610
return boost::asio::async_initiate<decltype(consigned), WatchSig>(
16261611
[timeout, this](auto&& handler, Object o, IOContext ioc, WatchCB cb) {
16271612
watch_(std::move(o), std::move(ioc), timeout, std::move(cb),
@@ -1633,9 +1618,7 @@ class RADOS final
16331618
auto watch(Object o, IOContext ioc, CompletionToken&& token,
16341619
std::optional<std::chrono::seconds> timeout = std::nullopt,
16351620
std::uint32_t queue_size = 128u) {
1636-
auto consigned = boost::asio::consign(
1637-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1638-
boost::asio::get_associated_executor(token, get_executor())));
1621+
auto consigned = consign(std::forward<CompletionToken>(token));
16391622
return boost::asio::async_initiate<decltype(consigned), WatchSig>(
16401623
[timeout, queue_size, this]
16411624
(auto&& handler, Object o, IOContext ioc) mutable {
@@ -1651,9 +1634,7 @@ class RADOS final
16511634
template<boost::asio::completion_token_for<
16521635
NextNotificationSig> CompletionToken>
16531636
auto next_notification(uint64_t cookie, CompletionToken&& token) {
1654-
auto consigned = boost::asio::consign(
1655-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1656-
boost::asio::get_associated_executor(token, get_executor())));
1637+
auto consigned = consign(std::forward<CompletionToken>(token));
16571638
return boost::asio::async_initiate<
16581639
decltype(consigned), NextNotificationSig>(
16591640
[cookie, this](auto&& handler) mutable {
@@ -1666,9 +1647,7 @@ class RADOS final
16661647
uint64_t notify_id, uint64_t cookie,
16671648
ceph::buffer::list bl,
16681649
CompletionToken&& token) {
1669-
auto consigned = boost::asio::consign(
1670-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1671-
boost::asio::get_associated_executor(token, get_executor())));
1650+
auto consigned = consign(std::forward<CompletionToken>(token));
16721651
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
16731652
[notify_id, cookie, this](auto&& handler, Object o, IOContext ioc,
16741653
buffer::list bl) {
@@ -1680,9 +1659,7 @@ class RADOS final
16801659
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
16811660
auto unwatch(std::uint64_t cookie, IOContext ioc,
16821661
CompletionToken&& token) {
1683-
auto consigned = boost::asio::consign(
1684-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1685-
boost::asio::get_associated_executor(token, get_executor())));
1662+
auto consigned = consign(std::forward<CompletionToken>(token));
16861663
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
16871664
[cookie, this](auto&& handler, IOContext ioc) {
16881665
unwatch_(cookie, std::move(ioc), std::move(handler));
@@ -1697,9 +1674,7 @@ class RADOS final
16971674
using VoidOpComp = boost::asio::any_completion_handler<VoidOpSig>;
16981675
template<boost::asio::completion_token_for<VoidOpSig> CompletionToken>
16991676
auto flush_watch(CompletionToken&& token) {
1700-
auto consigned = boost::asio::consign(
1701-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1702-
boost::asio::get_associated_executor(token, get_executor())));
1677+
auto consigned = consign(std::forward<CompletionToken>(token));
17031678
return boost::asio::async_initiate<decltype(consigned), VoidOpSig>(
17041679
[this](auto&& handler) {
17051680
flush_watch_(std::move(handler));
@@ -1720,9 +1695,7 @@ class RADOS final
17201695
auto notify(Object o, IOContext ioc, ceph::buffer::list bl,
17211696
std::optional<std::chrono::seconds> timeout,
17221697
CompletionToken&& token) {
1723-
auto consigned = boost::asio::consign(
1724-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1725-
boost::asio::get_associated_executor(token, get_executor())));
1698+
auto consigned = consign(std::forward<CompletionToken>(token));
17261699
return boost::asio::async_initiate<decltype(consigned), NotifySig>(
17271700
[timeout, this](auto&& handler, Object o, IOContext ioc,
17281701
buffer::list bl) {
@@ -1742,9 +1715,7 @@ class RADOS final
17421715
Cursor end, std::uint32_t max,
17431716
ceph::buffer::list filter,
17441717
CompletionToken&& token) {
1745-
auto consigned = boost::asio::consign(
1746-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1747-
boost::asio::get_associated_executor(token, get_executor())));
1718+
auto consigned = consign(std::forward<CompletionToken>(token));
17481719
return boost::asio::async_initiate<decltype(consigned), EnumerateSig>(
17491720
[max, this](auto&& handler, IOContext ioc, Cursor begin, Cursor end,
17501721
buffer::list filter) {
@@ -1760,9 +1731,7 @@ class RADOS final
17601731
template<boost::asio::completion_token_for<CommandSig> CompletionToken>
17611732
auto osd_command(int osd, std::vector<std::string> cmd,
17621733
ceph::buffer::list in, CompletionToken&& token) {
1763-
auto consigned = boost::asio::consign(
1764-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1765-
boost::asio::get_associated_executor(token, get_executor())));
1734+
auto consigned = consign(std::forward<CompletionToken>(token));
17661735
return boost::asio::async_initiate<decltype(consigned), CommandSig>(
17671736
[osd, this](auto&& handler, std::vector<std::string> cmd,
17681737
buffer::list in) {
@@ -1773,9 +1742,7 @@ class RADOS final
17731742
template<boost::asio::completion_token_for<CommandSig> CompletionToken>
17741743
auto pg_command(PG pg, std::vector<std::string> cmd,
17751744
ceph::buffer::list in, CompletionToken&& token) {
1776-
auto consigned = boost::asio::consign(
1777-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1778-
boost::asio::get_associated_executor(token, get_executor())));
1745+
auto consigned = consign(std::forward<CompletionToken>(token));
17791746
return boost::asio::async_initiate<decltype(consigned), CommandSig>(
17801747
[this](auto&& handler, PG pg, std::vector<std::string> cmd,
17811748
buffer::list in) {
@@ -1789,9 +1756,7 @@ class RADOS final
17891756
ceph::buffer::list bl,
17901757
std::string* outs, ceph::buffer::list* outbl,
17911758
CompletionToken&& token) {
1792-
auto consigned = boost::asio::consign(
1793-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1794-
boost::asio::get_associated_executor(token, get_executor())));
1759+
auto consigned = consign(std::forward<CompletionToken>(token));
17951760
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
17961761
[outs, outbl, this](auto&& handler, std::vector<std::string> command,
17971762
buffer::list bl) {
@@ -1803,9 +1768,7 @@ class RADOS final
18031768
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
18041769
auto enable_application(std::string pool, std::string app_name,
18051770
bool force, CompletionToken&& token) {
1806-
auto consigned = boost::asio::consign(
1807-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1808-
boost::asio::get_associated_executor(token, get_executor())));
1771+
auto consigned = consign(std::forward<CompletionToken>(token));
18091772
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
18101773
[force, this](auto&& handler, std::string pool, std::string app_name) {
18111774
enable_application_(std::move(pool), std::move(app_name), force,
@@ -1817,9 +1780,7 @@ class RADOS final
18171780
auto blocklist_add(std::string client_address,
18181781
std::optional<std::chrono::seconds> expire,
18191782
CompletionToken&& token) {
1820-
auto consigned = boost::asio::consign(
1821-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1822-
boost::asio::get_associated_executor(token, get_executor())));
1783+
auto consigned = consign(std::forward<CompletionToken>(token));
18231784
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
18241785
[expire, this](auto&& handler, std::string client_address) {
18251786
blocklist_add_(std::move(client_address), expire,
@@ -1829,9 +1790,7 @@ class RADOS final
18291790

18301791
template<boost::asio::completion_token_for<SimpleOpSig> CompletionToken>
18311792
auto wait_for_latest_osd_map(CompletionToken&& token) {
1832-
auto consigned = boost::asio::consign(
1833-
std::forward<CompletionToken>(token), boost::asio::make_work_guard(
1834-
boost::asio::get_associated_executor(token, get_executor())));
1793+
auto consigned = consign(std::forward<CompletionToken>(token));
18351794
return boost::asio::async_initiate<decltype(consigned), SimpleOpSig>(
18361795
[this](auto&& handler) {
18371796
wait_for_latest_osd_map_(std::move(handler));
@@ -1846,7 +1805,7 @@ class RADOS final
18461805

18471806
friend Builder;
18481807

1849-
RADOS(std::unique_ptr<detail::Client> impl);
1808+
RADOS(std::shared_ptr<detail::Client> impl);
18501809
static void make_with_cct_(CephContext* cct,
18511810
boost::asio::io_context& ioctx,
18521811
BuildComp c);
@@ -1930,7 +1889,7 @@ class RADOS final
19301889
void wait_for_latest_osd_map_(SimpleOpComp c);
19311890

19321891
// Proxy object to provide access to low-level RADOS messaging clients
1933-
std::unique_ptr<detail::Client> impl;
1892+
std::shared_ptr<detail::Client> impl;
19341893
};
19351894
#pragma clang diagnostic pop
19361895

0 commit comments

Comments
 (0)