Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

rgw leaks fixes #8636

Merged
merged 5 commits into from Apr 19, 2016
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/rgw/rgw_coroutine.cc
Expand Up @@ -803,6 +803,19 @@ void RGWCoroutine::dump(Formatter *f) const {
}
}

RGWSimpleCoroutine::~RGWSimpleCoroutine()
{
if (!called_cleanup) {
request_cleanup();
}
}

void RGWSimpleCoroutine::call_cleanup()
{
called_cleanup = true;
request_cleanup();
}

int RGWSimpleCoroutine::operate()
{
int ret = 0;
Expand All @@ -812,6 +825,7 @@ int RGWSimpleCoroutine::operate()
yield return state_request_complete();
yield return state_all_complete();
drain_all();
call_cleanup();
return set_state(RGWCoroutine_Done, ret);
}
return 0;
Expand All @@ -821,6 +835,7 @@ int RGWSimpleCoroutine::state_init()
{
int ret = init();
if (ret < 0) {
call_cleanup();
return set_state(RGWCoroutine_Error, ret);
}
return 0;
Expand All @@ -830,6 +845,7 @@ int RGWSimpleCoroutine::state_send_request()
{
int ret = send_request();
if (ret < 0) {
call_cleanup();
return set_state(RGWCoroutine_Error, ret);
}
return io_block(0);
Expand All @@ -839,6 +855,7 @@ int RGWSimpleCoroutine::state_request_complete()
{
int ret = request_complete();
if (ret < 0) {
call_cleanup();
return set_state(RGWCoroutine_Error, ret);
}
return 0;
Expand All @@ -848,6 +865,7 @@ int RGWSimpleCoroutine::state_all_complete()
{
int ret = finish();
if (ret < 0) {
call_cleanup();
return set_state(RGWCoroutine_Error, ret);
}
return 0;
Expand Down
15 changes: 11 additions & 4 deletions src/rgw/rgw_coroutine.h
Expand Up @@ -536,6 +536,7 @@ class RGWCoroutinesManager {
}
}
virtual ~RGWCoroutinesManager() {
stop();
completion_mgr->put();
if (cr_registry) {
cr_registry->remove(this);
Expand All @@ -545,8 +546,9 @@ class RGWCoroutinesManager {
int run(list<RGWCoroutinesStack *>& ops);
int run(RGWCoroutine *op);
void stop() {
going_down.set(1);
completion_mgr->go_down();
if (going_down.inc() == 1) {
completion_mgr->go_down();
}
}

virtual void report_error(RGWCoroutinesStack *op);
Expand All @@ -562,21 +564,26 @@ class RGWCoroutinesManager {
};

class RGWSimpleCoroutine : public RGWCoroutine {
bool called_cleanup;

int operate();

int state_init();
int state_send_request();
int state_request_complete();
int state_all_complete();

void call_cleanup();

public:
RGWSimpleCoroutine(CephContext *_cct) : RGWCoroutine(_cct) {}
RGWSimpleCoroutine(CephContext *_cct) : RGWCoroutine(_cct), called_cleanup(false) {}
~RGWSimpleCoroutine();

virtual int init() { return 0; }
virtual int send_request() = 0;
virtual int request_complete() = 0;
virtual int finish() { return 0; }

virtual void request_cleanup() {}
};

#endif
6 changes: 3 additions & 3 deletions src/rgw/rgw_cr_rados.cc
Expand Up @@ -305,7 +305,7 @@ RGWSimpleRadosLockCR::RGWSimpleRadosLockCR(RGWAsyncRadosProcessor *_async_rados,
s << "rados lock dest=" << pool << "/" << oid << " lock=" << lock_name << " cookie=" << cookie << " duration=" << duration;
}

RGWSimpleRadosLockCR::~RGWSimpleRadosLockCR()
void RGWSimpleRadosLockCR::request_cleanup()
{
if (req) {
req->finish();
Expand Down Expand Up @@ -341,7 +341,7 @@ RGWSimpleRadosUnlockCR::RGWSimpleRadosUnlockCR(RGWAsyncRadosProcessor *_async_ra
set_description() << "rados unlock dest=" << pool << "/" << oid << " lock=" << lock_name << " cookie=" << cookie;
}

RGWSimpleRadosUnlockCR::~RGWSimpleRadosUnlockCR()
void RGWSimpleRadosUnlockCR::request_cleanup()
{
if (req) {
req->finish();
Expand Down Expand Up @@ -635,7 +635,7 @@ RGWStatObjCR::RGWStatObjCR(RGWAsyncRadosProcessor *async_rados, RGWRados *store,
{
}

RGWStatObjCR::~RGWStatObjCR()
void RGWStatObjCR::request_cleanup()
{
if (req) {
req->finish();
Expand Down
31 changes: 18 additions & 13 deletions src/rgw/rgw_cr_rados.h
Expand Up @@ -9,7 +9,6 @@ class RGWAsyncRadosRequest : public RefCountedObject {
RGWCoroutine *caller;
RGWAioCompletionNotifier *notifier;

void *user_info;
int retcode;

bool done;
Expand All @@ -19,22 +18,26 @@ class RGWAsyncRadosRequest : public RefCountedObject {
protected:
virtual int _send_request() = 0;
public:
RGWAsyncRadosRequest(RGWCoroutine *_caller, RGWAioCompletionNotifier *_cn) : caller(_caller), notifier(_cn),
RGWAsyncRadosRequest(RGWCoroutine *_caller, RGWAioCompletionNotifier *_cn) : caller(_caller), notifier(_cn), retcode(0),
done(false), lock("RGWAsyncRadosRequest::lock") {
notifier->get();
caller->get();
}
virtual ~RGWAsyncRadosRequest() {
notifier->put();
caller->put();
}

void send_request() {
get();
retcode = _send_request();
{
Mutex::Locker l(lock);
if (!done) {
notifier->cb();
}
}
put();
}

int get_ret_status() { return retcode; }
Expand Down Expand Up @@ -195,7 +198,7 @@ class RGWSimpleRadosReadCR : public RGWSimpleCoroutine {
result(_result),
req(NULL) { }

~RGWSimpleRadosReadCR() {
void request_cleanup() {
if (req) {
req->finish();
}
Expand Down Expand Up @@ -270,7 +273,7 @@ class RGWSimpleRadosReadAttrsCR : public RGWSimpleCoroutine {
pattrs(_pattrs),
req(NULL) { }

~RGWSimpleRadosReadAttrsCR() {
void request_cleanup() {
if (req) {
req->finish();
}
Expand Down Expand Up @@ -302,7 +305,7 @@ class RGWSimpleRadosWriteCR : public RGWSimpleCoroutine {
::encode(_data, bl);
}

~RGWSimpleRadosWriteCR() {
void request_cleanup() {
if (req) {
req->finish();
}
Expand Down Expand Up @@ -342,7 +345,7 @@ class RGWSimpleRadosWriteAttrsCR : public RGWSimpleCoroutine {
attrs(_attrs), req(NULL) {
}

~RGWSimpleRadosWriteAttrsCR() {
void request_cleanup() {
if (req) {
req->finish();
}
Expand Down Expand Up @@ -374,6 +377,7 @@ class RGWRadosSetOmapKeysCR : public RGWSimpleCoroutine {
RGWRadosSetOmapKeysCR(RGWRados *_store,
rgw_bucket& _pool, const string& _oid,
map<string, bufferlist>& _entries);

~RGWRadosSetOmapKeysCR();

int send_request();
Expand All @@ -400,6 +404,7 @@ class RGWRadosGetOmapKeysCR : public RGWSimpleCoroutine {
const rgw_bucket& _pool, const string& _oid,
const string& _marker,
map<string, bufferlist> *_entries, int _max_entries);

~RGWRadosGetOmapKeysCR();

int send_request();
Expand All @@ -426,7 +431,7 @@ class RGWSimpleRadosLockCR : public RGWSimpleCoroutine {
const rgw_bucket& _pool, const string& _oid, const string& _lock_name,
const string& _cookie,
uint32_t _duration);
~RGWSimpleRadosLockCR();
void request_cleanup();

int send_request();
int request_complete();
Expand All @@ -447,7 +452,7 @@ class RGWSimpleRadosUnlockCR : public RGWSimpleCoroutine {
RGWSimpleRadosUnlockCR(RGWAsyncRadosProcessor *_async_rados, RGWRados *_store,
const rgw_bucket& _pool, const string& _oid, const string& _lock_name,
const string& _cookie);
~RGWSimpleRadosUnlockCR();
void request_cleanup();

int send_request();
int request_complete();
Expand Down Expand Up @@ -518,7 +523,7 @@ class RGWWaitCR : public RGWSimpleCoroutine {
async_rados(_async_rados), lock(_lock), cond(_cond), secs(_secs), req(NULL) {
}

~RGWWaitCR() {
void request_cleanup() {
wakeup();
if (req) {
req->finish();
Expand Down Expand Up @@ -616,7 +621,7 @@ class RGWGetBucketInstanceInfoCR : public RGWSimpleCoroutine {
RGWBucketInfo *_bucket_info) : RGWSimpleCoroutine(_store->ctx()), async_rados(_async_rados), store(_store),
bucket_name(_bucket_name), bucket_id(_bucket_id),
bucket_info(_bucket_info), req(NULL) {}
~RGWGetBucketInstanceInfoCR() {
void request_cleanup() {
if (req) {
req->finish();
}
Expand Down Expand Up @@ -693,7 +698,7 @@ class RGWFetchRemoteObjCR : public RGWSimpleCoroutine {
copy_if_newer(_if_newer), req(NULL) {}


~RGWFetchRemoteObjCR() {
void request_cleanup() {
if (req) {
req->finish();
}
Expand Down Expand Up @@ -808,7 +813,7 @@ class RGWRemoveObjCR : public RGWSimpleCoroutine {
}
}

~RGWRemoveObjCR() {
void request_cleanup() {
if (req) {
req->finish();
}
Expand Down Expand Up @@ -931,7 +936,7 @@ class RGWStatObjCR : public RGWSimpleCoroutine {
const rgw_obj& obj, uint64_t *psize = nullptr,
real_time* pmtime = nullptr, uint64_t *pepoch = nullptr,
RGWObjVersionTracker *objv_tracker = nullptr);
~RGWStatObjCR();
void request_cleanup();

int send_request() override;
int request_complete() override;
Expand Down
14 changes: 14 additions & 0 deletions src/rgw/rgw_cr_rest.h
Expand Up @@ -51,6 +51,12 @@ class RGWReadRESTResourceCR : public RGWSimpleCoroutine {
}
return 0;
}

void request_cleanup() {
if (http_op) {
http_op->put();
}
}
};

template <class S, class T>
Expand Down Expand Up @@ -89,6 +95,7 @@ class RGWPostRESTResourceCR : public RGWSimpleCoroutine {
int ret = op->aio_send(bl);
if (ret < 0) {
lsubdout(cct, rgw, 0) << "ERROR: failed to send post request" << dendl;
op->put();
return ret;
}
std::swap(http_op, op); // store reference in http_op on success
Expand All @@ -109,10 +116,17 @@ class RGWPostRESTResourceCR : public RGWSimpleCoroutine {
<< " status=" << op->get_http_status() << std::endl;
lsubdout(cct, rgw, 0) << "ERROR: failed to wait for op, ret=" << ret
<< ": " << op->to_str() << dendl;
op->put();
return ret;
}
return 0;
}

void request_cleanup() {
if (http_op) {
http_op->put();
}
}
};

#endif
7 changes: 6 additions & 1 deletion src/rgw/rgw_realm_watcher.cc
Expand Up @@ -124,7 +124,12 @@ int RGWRealmWatcher::watch_start(RGWRealm& realm)
int RGWRealmWatcher::watch_restart()
{
assert(!watch_oid.empty());
int r = pool_ctx.watch2(watch_oid, &watch_handle, this);
int r = pool_ctx.unwatch2(watch_handle);
if (r < 0) {
lderr(cct) << "Failed to unwatch on " << watch_oid
<< " with " << cpp_strerror(-r) << dendl;
}
r = pool_ctx.watch2(watch_oid, &watch_handle, this);
if (r < 0)
lderr(cct) << "Failed to restart watch on " << watch_oid
<< " with " << cpp_strerror(-r) << dendl;
Expand Down