Bug #10425
RadosClient::connect leaks Objecter if called twice
0%
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.
Associated revisions
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>
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 about 6 years ago
I am working on this bug.
#2 Updated by Loïc Dachary about 6 years ago
- Status changed from 12 to In Progress
- Assignee set to Loïc Dachary
Cool, thanks. I assign it to myself to track this, let me know if you need any help.
#3 Updated by Radoslaw Zarzynski about 6 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?
#4 Updated by Radoslaw Zarzynski about 6 years ago
Link to pull request: https://github.com/ceph/ceph/pull/3513
#5 Updated by Loïc Dachary about 6 years ago
#6 Updated by Loïc Dachary about 6 years ago
- Backport set to dumpling,firefly
#7 Updated by Loïc Dachary about 6 years ago
- Status changed from In Progress to 7
#8 Updated by Loïc Dachary about 6 years ago
- Status changed from 7 to Pending Backport
#9 Updated by Loïc Dachary about 6 years ago
- Backport changed from dumpling,firefly to firefly
dumpling is end of life
#10 Updated by Loïc Dachary about 6 years ago
1b26672 librados: fix resources leakage in RadosClient::connect(). (in firefly),
#11 Updated by Loïc Dachary about 6 years ago
- Status changed from Pending Backport to Resolved