Skip to content

Commit

Permalink
Objecter: check the 'initialized' atomic_t safely
Browse files Browse the repository at this point in the history
shutdown() resets initialized to 0, but we can still receive messages
after this point, so fix message handlers to skip messages in this
case instead of asserting.

Also read initialized while holding Objecter::rwlock to avoid races
where e.g. handle_osd_map() checks initialized -> 1, continues,
shutdown() is called, sets initialized to 0, then handle_osd_map()
goes about its business and calls op_submit(), which would fail the
assert(initialized.read()) check. Similar races existed in other
message handlers which change Objecter state.

The Objecter is not destroyed until after its Messenger in
the MDS, OSD, and librados, so this should be safe.

Fixes: #9617
Backport: giant
Signed-off-by: Josh Durgin <josh.durgin@inktank.com>
(cherry picked from commit e506f89)

Conflicts:
	src/osdc/Objecter.cc
        context changed: Objecter::tick() did not have
        assert(initialized.read())
  • Loading branch information
jdurgin authored and ldachary committed Mar 19, 2015
1 parent ce436a3 commit d790833
Showing 1 changed file with 31 additions and 8 deletions.
39 changes: 31 additions & 8 deletions src/osdc/Objecter.cc
Expand Up @@ -721,9 +721,9 @@ void Objecter::_scan_requests(OSDSession *s,

void Objecter::handle_osd_map(MOSDMap *m)
{
assert(initialized.read());

RWLock::WLocker wl(rwlock);
if (!initialized.read())
return;

assert(osdmap);

Expand Down Expand Up @@ -1548,6 +1548,8 @@ void Objecter::tick()
RWLock::RLocker rl(rwlock);

ldout(cct, 10) << "tick" << dendl;
if (!initialized.read())
return;

// we are only called by C_Tick
assert(tick_event);
Expand Down Expand Up @@ -2513,7 +2515,6 @@ void Objecter::unregister_op(Op *op)
/* This function DOES put the passed message before returning */
void Objecter::handle_osd_op_reply(MOSDOpReply *m)
{
assert(initialized.read());
ldout(cct, 10) << "in handle_osd_op_reply" << dendl;

// get pio
Expand All @@ -2522,6 +2523,11 @@ void Objecter::handle_osd_op_reply(MOSDOpReply *m)
int osd_num = (int)m->get_source().num();

RWLock::RLocker l(rwlock);
if (!initialized.read()) {
m->put();
return;
}

RWLock::Context lc(rwlock, RWLock::Context::TakenForRead);

map<int, OSDSession *>::iterator siter = osd_sessions.find(osd_num);
Expand Down Expand Up @@ -3098,9 +3104,12 @@ void Objecter::_pool_op_submit(PoolOp *op)
*/
void Objecter::handle_pool_op_reply(MPoolOpReply *m)
{
assert(initialized.read());

rwlock.get_read();
if (!initialized.read()) {
rwlock.put_read();
m->put();
return;
}

ldout(cct, 10) << "handle_pool_op_reply " << *m << dendl;
ceph_tid_t tid = m->get_tid();
Expand Down Expand Up @@ -3229,11 +3238,15 @@ void Objecter::_poolstat_submit(PoolStatOp *op)

void Objecter::handle_get_pool_stats_reply(MGetPoolStatsReply *m)
{
assert(initialized.read());
ldout(cct, 10) << "handle_get_pool_stats_reply " << *m << dendl;
ceph_tid_t tid = m->get_tid();

RWLock::WLocker wl(rwlock);
if (!initialized.read()) {
m->put();
return;
}

map<ceph_tid_t, PoolStatOp *>::iterator iter = poolstat_ops.find(tid);
if (iter != poolstat_ops.end()) {
PoolStatOp *op = poolstat_ops[tid];
Expand Down Expand Up @@ -3334,9 +3347,11 @@ void Objecter::_fs_stats_submit(StatfsOp *op)

void Objecter::handle_fs_stats_reply(MStatfsReply *m)
{
assert(initialized.read());

RWLock::WLocker wl(rwlock);
if (!initialized.read()) {
m->put();
return;
}

ldout(cct, 10) << "handle_fs_stats_reply " << *m << dendl;
ceph_tid_t tid = m->get_tid();
Expand Down Expand Up @@ -3445,6 +3460,10 @@ bool Objecter::ms_handle_reset(Connection *con)
if (osd >= 0) {
ldout(cct, 1) << "ms_handle_reset on osd." << osd << dendl;
rwlock.get_write();
if (!initialized.read()) {
rwlock.put_write();
return false;
}
map<int,OSDSession*>::iterator p = osd_sessions.find(osd);
if (p != osd_sessions.end()) {
OSDSession *session = p->second;
Expand Down Expand Up @@ -3756,6 +3775,10 @@ void Objecter::handle_command_reply(MCommandReply *m)
int osd_num = (int)m->get_source().num();

RWLock::WLocker wl(rwlock);
if (!initialized.read()) {
m->put();
return;
}

map<int, OSDSession *>::iterator siter = osd_sessions.find(osd_num);
if (siter == osd_sessions.end()) {
Expand Down

0 comments on commit d790833

Please sign in to comment.