Bug #10425: RadosClient::connect leaks Objecter if called twice - Ceph - Ceph

Bug #10425

RadosClient::connect leaks Objecter if called twice

Added by Loic Dachary almost 5 years ago. Updated over 4 years ago.

Status:

Resolved

Priority:

Normal

Assignee:

Loic Dachary

Category:

librados

Target version:

% Done:

Source:

other

Tags:

Backport:

firefly

Regression:

Severity:

3 - minor

Reviewed:

Affected Versions:

ceph-qa-suite:

Pull request ID:

Crash signature:

Description

http://workbench.dachary.org/ceph/ceph/blob/giant/src/librados/RadosClient.cc#L226

If RadosClient::connect is called a second time (maybe after fixing an error condition), the Objecter allocated by the first call will be leaked.

20150121_10425_fix_v1.patch View (1.06 KB) Radoslaw Zarzynski, 01/21/2015 12:04 PM

Associated revisions

Revision 624c056d (diff)
Added by Radoslaw Zarzynski almost 5 years ago

librados: fix resources leakage in RadosClient::connect().

If RadosClient::connect was called a second time (which could
happen as a part of recovery from failure), the instances
of Objecter and Messenger allocated by the first call were leaked.

Additionally, the implementation of the method wrongly reported
memory allocation problems -- it throwed std::bad_alloc exception
instead of returning -ENOMEM error code.

Fixes: #10425
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>

Revision 1b266721 (diff)
Added by Radoslaw Zarzynski almost 5 years ago

librados: fix resources leakage in RadosClient::connect().

If RadosClient::connect was called a second time (which could
happen as a part of recovery from failure), the instances
of Objecter and Messenger allocated by the first call were leaked.

Additionally, the implementation of the method wrongly reported
memory allocation problems -- it throwed std::bad_alloc exception
instead of returning -ENOMEM error code.

Fixes: #10425
Signed-off-by: Radoslaw Zarzynski <rzarzynski@mirantis.com>
(cherry picked from commit 624c056da093c8741242892413438a291c03c7d5)

Conflicts:
src/librados/RadosClient.cc
resolve adding (std::nothrow) that failed because the
prototype of the constructor is not the same

History

#1 Updated by Radoslaw Zarzynski almost 5 years ago

I am working on this bug.

#2 Updated by Loic Dachary almost 5 years ago

Status changed from 12 to In Progress
Assignee set to Loic Dachary

Cool, thanks. I assign it to myself to track this, let me know if you need any help.

#3 Updated by Radoslaw Zarzynski almost 5 years ago

File 20150121_10425_fix_v1.patch View added

I am attaching the first proposal of patch for your review.

Besides rectifying the Objecter leak, two additional things have been done in the body of RadosClient::connect method:

Switch from throwing variant of “new” to the non-throwing one. I think that handling memory allocation problems via std::bad_alloc exception was unintentional behavior, especially due to the usage of ENOMEM return code across the whole method. It might lead to stuck in CONNECTING state in some circumstances.
Leak of Messenger instance. The issue is very similar to the main problem so I am proposing to fix it together.

Could you please tell me about guard (objecter != NULL) on RadosClient::wait_for_osdmap method? Does the check need to base on assumption about objecter? Maybe the explicit state verification (like in other methods of the class) should be performed?