Skip to content

Commit 0908b59

Browse files
ljfloresljflores
authored
Merge pull request ceph#65698 from cbodley/wip-72771
osdc: Objecter::linger_by_cookie() for safe cast from uint64 Reviewed-by: J. Eric Ivancich <[email protected]>
2 parents 37140e8 + 94f42b6 commit 0908b59

File tree

4 files changed

+76
-36
lines changed

4 files changed

+76
-36
lines changed

‎src/librados/IoCtxImpl.cc‎

Lines changed: 29 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -89,25 +89,28 @@ struct CB_notify_Finish {
8989

9090
struct CB_aio_linger_cancel {
9191
Objecter *objecter;
92-
Objecter::LingerOp *linger_op;
92+
boost::intrusive_ptr<Objecter::LingerOp> linger_op;
9393

94-
CB_aio_linger_cancel(Objecter *_objecter, Objecter::LingerOp *_linger_op)
95-
: objecter(_objecter), linger_op(_linger_op)
94+
CB_aio_linger_cancel(Objecter *_objecter,
95+
boost::intrusive_ptr<Objecter::LingerOp> op)
96+
: objecter(_objecter), linger_op(std::move(op))
9697
{
9798
}
9899

99100
void operator()() {
100-
objecter->linger_cancel(linger_op);
101+
objecter->linger_cancel(linger_op.get());
101102
}
102103
};
103104

104105
struct C_aio_linger_Complete : public Context {
105106
AioCompletionImpl *c;
106-
Objecter::LingerOp *linger_op;
107+
boost::intrusive_ptr<Objecter::LingerOp> linger_op;
107108
bool cancel;
108109

109-
C_aio_linger_Complete(AioCompletionImpl *_c, Objecter::LingerOp *_linger_op, bool _cancel)
110-
: c(_c), linger_op(_linger_op), cancel(_cancel)
110+
C_aio_linger_Complete(AioCompletionImpl *_c,
111+
boost::intrusive_ptr<Objecter::LingerOp> op,
112+
bool _cancel)
113+
: c(_c), linger_op(std::move(op)), cancel(_cancel)
111114
{
112115
c->get();
113116
}
@@ -116,7 +119,7 @@ struct C_aio_linger_Complete : public Context {
116119
if (cancel || r < 0)
117120
boost::asio::defer(c->io->client->finish_strand,
118121
CB_aio_linger_cancel(c->io->objecter,
119-
linger_op));
122+
std::move(linger_op)));
120123

121124
c->lock.lock();
122125
c->rval = r;
@@ -137,8 +140,9 @@ struct C_aio_notify_Complete : public C_aio_linger_Complete {
137140
bool finished = false;
138141
int ret_val = 0;
139142

140-
C_aio_notify_Complete(AioCompletionImpl *_c, Objecter::LingerOp *_linger_op)
141-
: C_aio_linger_Complete(_c, _linger_op, false) {
143+
C_aio_notify_Complete(AioCompletionImpl *_c,
144+
boost::intrusive_ptr<Objecter::LingerOp> op)
145+
: C_aio_linger_Complete(_c, std::move(op), false) {
142146
}
143147

144148
void handle_ack(int r) {
@@ -1742,8 +1746,11 @@ int librados::IoCtxImpl::notify_ack(
17421746

17431747
int librados::IoCtxImpl::watch_check(uint64_t cookie)
17441748
{
1745-
auto linger_op = reinterpret_cast<Objecter::LingerOp*>(cookie);
1746-
auto r = objecter->linger_check(linger_op);
1749+
boost::intrusive_ptr linger_op = objecter->linger_by_cookie(cookie);
1750+
if (!linger_op) {
1751+
return -ENOTCONN;
1752+
}
1753+
auto r = objecter->linger_check(linger_op.get());
17471754
if (r)
17481755
return 1 + std::chrono::duration_cast<
17491756
std::chrono::milliseconds>(*r).count();
@@ -1753,7 +1760,11 @@ int librados::IoCtxImpl::watch_check(uint64_t cookie)
17531760

17541761
int librados::IoCtxImpl::unwatch(uint64_t cookie)
17551762
{
1756-
Objecter::LingerOp *linger_op = reinterpret_cast<Objecter::LingerOp*>(cookie);
1763+
boost::intrusive_ptr linger_op = objecter->linger_by_cookie(cookie);
1764+
if (!linger_op) {
1765+
return -ENOTCONN;
1766+
}
1767+
17571768
C_SaferCond onfinish;
17581769
version_t ver = 0;
17591770

@@ -1763,7 +1774,7 @@ int librados::IoCtxImpl::unwatch(uint64_t cookie)
17631774
objecter->mutate(linger_op->target.base_oid, oloc, wr,
17641775
snapc, ceph::real_clock::now(), extra_op_flags,
17651776
&onfinish, &ver);
1766-
objecter->linger_cancel(linger_op);
1777+
objecter->linger_cancel(linger_op.get());
17671778

17681779
int r = onfinish.wait();
17691780
set_sync_op_version(ver);
@@ -1773,7 +1784,10 @@ int librados::IoCtxImpl::unwatch(uint64_t cookie)
17731784
int librados::IoCtxImpl::aio_unwatch(uint64_t cookie, AioCompletionImpl *c)
17741785
{
17751786
c->io = this;
1776-
Objecter::LingerOp *linger_op = reinterpret_cast<Objecter::LingerOp*>(cookie);
1787+
boost::intrusive_ptr linger_op = objecter->linger_by_cookie(cookie);
1788+
if (!linger_op) {
1789+
return -ENOTCONN;
1790+
}
17771791
Context *oncomplete = new C_aio_linger_Complete(c, linger_op, true);
17781792

17791793
::ObjectOperation wr;

‎src/neorados/RADOS.cc‎

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1571,8 +1571,8 @@ void RADOS::watch_(Object o, IOContext _ioc, WatchComp c,
15711571
}
15721572

15731573
void RADOS::next_notification_(uint64_t cookie, NextNotificationComp c) {
1574-
Objecter::LingerOp* linger_op = reinterpret_cast<Objecter::LingerOp*>(cookie);
1575-
if (!impl->objecter->is_valid_watch(linger_op)) {
1574+
boost::intrusive_ptr linger_op = impl->objecter->linger_by_cookie(cookie);
1575+
if (!linger_op) {
15761576
dispatch(asio::append(std::move(c),
15771577
bs::error_code(ENOTCONN, bs::generic_category()),
15781578
Notification{}));
@@ -1612,9 +1612,9 @@ void RADOS::notify_ack_(Object o, IOContext _ioc,
16121612

16131613
tl::expected<ceph::timespan, bs::error_code> RADOS::check_watch(uint64_t cookie)
16141614
{
1615-
auto linger_op = reinterpret_cast<Objecter::LingerOp*>(cookie);
1616-
if (impl->objecter->is_valid_watch(linger_op)) {
1617-
return impl->objecter->linger_check(linger_op);
1615+
boost::intrusive_ptr linger_op = impl->objecter->linger_by_cookie(cookie);
1616+
if (linger_op) {
1617+
return impl->objecter->linger_check(linger_op.get());
16181618
} else {
16191619
return tl::unexpected(bs::error_code(ENOTCONN, bs::generic_category()));
16201620
}
@@ -1625,7 +1625,12 @@ void RADOS::unwatch_(uint64_t cookie, IOContext _ioc,
16251625
{
16261626
auto ioc = reinterpret_cast<const IOContextImpl*>(&_ioc.impl);
16271627

1628-
Objecter::LingerOp *linger_op = reinterpret_cast<Objecter::LingerOp*>(cookie);
1628+
boost::intrusive_ptr linger_op = impl->objecter->linger_by_cookie(cookie);
1629+
if (!linger_op) {
1630+
dispatch(asio::append(std::move(c),
1631+
bs::error_code(ENOTCONN, bs::generic_category())));
1632+
return;
1633+
}
16291634

16301635
ObjectOperation op;
16311636
op.watch(cookie, CEPH_OSD_WATCH_OP_UNWATCH);
@@ -1638,7 +1643,7 @@ void RADOS::unwatch_(uint64_t cookie, IOContext _ioc,
16381643
[objecter = impl->objecter,
16391644
linger_op, c = std::move(c)]
16401645
(bs::error_code ec) mutable {
1641-
objecter->linger_cancel(linger_op);
1646+
objecter->linger_cancel(linger_op.get());
16421647
asio::dispatch(asio::append(std::move(c), ec));
16431648
}));
16441649
}

‎src/osdc/Objecter.cc‎

Lines changed: 27 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -658,9 +658,10 @@ class CB_DoWatchError {
658658
boost::intrusive_ptr<Objecter::LingerOp> info;
659659
bs::error_code ec;
660660
public:
661-
CB_DoWatchError(Objecter *o, Objecter::LingerOp *i,
661+
CB_DoWatchError(Objecter *o,
662+
boost::intrusive_ptr<Objecter::LingerOp> i,
662663
bs::error_code ec)
663-
: objecter(o), info(i), ec(ec) {
664+
: objecter(o), info(std::move(i)), ec(ec) {
664665
info->_queued_async();
665666
}
666667
void operator()() {
@@ -813,7 +814,22 @@ void Objecter::_linger_cancel(LingerOp *info)
813814
}
814815
}
815816

817+
auto Objecter::linger_by_cookie(uint64_t cookie)
818+
-> boost::intrusive_ptr<LingerOp>
819+
{
820+
auto lock = std::shared_lock{rwlock};
821+
return _linger_by_cookie(cookie);
822+
}
816823

824+
auto Objecter::_linger_by_cookie(uint64_t cookie)
825+
-> boost::intrusive_ptr<LingerOp>
826+
{
827+
auto info = reinterpret_cast<LingerOp*>(cookie);
828+
if (!linger_ops_set.contains(info)) {
829+
return nullptr; // invalid or canceled op
830+
}
831+
return info;
832+
}
817833

818834
Objecter::LingerOp *Objecter::linger_register(const object_t& oid,
819835
const object_locator_t& oloc,
@@ -920,8 +936,10 @@ struct CB_DoWatchNotify {
920936
Objecter *objecter;
921937
boost::intrusive_ptr<Objecter::LingerOp> info;
922938
boost::intrusive_ptr<MWatchNotify> msg;
923-
CB_DoWatchNotify(Objecter *o, Objecter::LingerOp *i, MWatchNotify *m)
924-
: objecter(o), info(i), msg(m) {
939+
CB_DoWatchNotify(Objecter *o,
940+
boost::intrusive_ptr<Objecter::LingerOp> i,
941+
MWatchNotify *m)
942+
: objecter(o), info(std::move(i)), msg(m) {
925943
info->_queued_async();
926944
}
927945
void operator()() {
@@ -936,8 +954,8 @@ void Objecter::handle_watch_notify(MWatchNotify *m)
936954
return;
937955
}
938956

939-
LingerOp *info = reinterpret_cast<LingerOp*>(m->cookie);
940-
if (linger_ops_set.count(info) == 0) {
957+
boost::intrusive_ptr info = _linger_by_cookie(m->cookie);
958+
if (!info) {
941959
ldout(cct, 7) << __func__ << " cookie " << m->cookie << " dne" << dendl;
942960
return;
943961
}
@@ -946,8 +964,8 @@ void Objecter::handle_watch_notify(MWatchNotify *m)
946964
if (!info->last_error) {
947965
info->last_error = bs::error_code(ENOTCONN, osd_category());
948966
if (info->handle) {
949-
asio::defer(finish_strand, CB_DoWatchError(this, info,
950-
info->last_error));
967+
boost::system::error_code ec = info->last_error;
968+
asio::defer(finish_strand, CB_DoWatchError(this, std::move(info), ec));
951969
}
952970
}
953971
} else if (!info->is_watch) {
@@ -968,7 +986,7 @@ void Objecter::handle_watch_notify(MWatchNotify *m)
968986
info->on_notify_finish = nullptr;
969987
}
970988
} else {
971-
asio::defer(finish_strand, CB_DoWatchNotify(this, info, m));
989+
asio::defer(finish_strand, CB_DoWatchNotify(this, std::move(info), m));
972990
}
973991
}
974992

‎src/osdc/Objecter.h‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2593,11 +2593,6 @@ class Objecter : public md_config_obs_t, public Dispatcher {
25932593
friend class CB_DoWatchError;
25942594
public:
25952595

2596-
bool is_valid_watch(LingerOp* op) {
2597-
std::shared_lock l(rwlock);
2598-
return linger_ops_set.contains(op);
2599-
}
2600-
26012596
template<typename CT>
26022597
auto linger_callback_flush(CT&& ct) {
26032598
auto consigned = boost::asio::consign(
@@ -3254,6 +3249,14 @@ class Objecter : public md_config_obs_t, public Dispatcher {
32543249
void linger_cancel(LingerOp *info); // releases a reference
32553250
void _linger_cancel(LingerOp *info);
32563251

3252+
// return the LingerOp associated with the given cookie.
3253+
// may return nullptr if the cookie is no longer valid
3254+
boost::intrusive_ptr<LingerOp> linger_by_cookie(uint64_t cookie);
3255+
private:
3256+
// internal version that expects the caller to hold rwlock
3257+
boost::intrusive_ptr<LingerOp> _linger_by_cookie(uint64_t cookie);
3258+
public:
3259+
32573260
void _do_watch_notify(boost::intrusive_ptr<LingerOp> info,
32583261
boost::intrusive_ptr<MWatchNotify> m);
32593262

0 commit comments

Comments
 (0)