-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/14555/#review26815
-----------------------------------------------------------
int local()
{
try {
boost::intrusive_ptr<DataTokenImpl> dtokp(new DataTokenImpl);
dtokp->addRef();
I believe the above line is the issue, not boost itself. This is adding an
*extra* reference over and above the one added automatically through the dtokp
pointer instance. The count will never reach zero unless there is a
corresponding *manual* release to offset this manual increment of the reference
count.
enqueue_data_record(dtokp.get());
}
catch (int e) {
std::cout << "local: caught exception " << e << std::endl;
throw;
}
return 0;
}
- Gordon Sim
On Oct. 9, 2013, 9:42 a.m., Pavel Moravec wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/14555/
> -----------------------------------------------------------
>
> (Updated Oct. 9, 2013, 9:42 a.m.)
>
>
> Review request for qpid, Gordon Sim and Kim van der Riet.
>
>
> Bugs: https://issues.apache.org/jira/browse/QPID-5214
>
> https://issues.apache.org/jira/browse/https://issues.apache.org/jira/browse/QPID-5214
>
>
> Repository: qpid
>
>
> Description
> -------
>
> mrg::msgstore::MessageStoreImpl::enqueue method calls:
>
> ..
> if (queue) {
> boost::intrusive_ptr<DataTokenImpl> dtokp(new DataTokenImpl);
> dtokp->addRef();
> ..
> (where in further code calls executed, an exception is raised within the
> scope of dtokp life)
>
> Apparently, boost library has a limitation with throwing exceptions where it
> forgets to remove some reference to the object, causing delete is not
> executed when intrusive_ptr variable is gone. See my reproducer below,
> independend on qpid/store code.
>
> Therefore, to workaround it, let release the dtokp manually just before
> throwing the exception. But I don't fully like adding new parameter to
> handleIoResult method (and not sure with re-casting dtokp from data_tok* to
> DataTokenImpl* is done in proper way).
>
> Reproducer of boost::intrusive_ptr limitation when throwing exception: let
> run below program via valgrind, first as is (direct memory loss), and then
> with "dtokp->release();" line uncommented (no direct memory loss).
>
> #include <iostream>
> #include <boost/intrusive_ptr.hpp>
> #include <boost/utility.hpp>
> #include <boost/detail/atomic_count.hpp>
>
> class RefCounted : public boost::noncopyable {
> mutable boost::detail::atomic_count count;
>
> public:
> RefCounted() : count(0) {}
> void addRef() const { ++count; }
> void release() const { if (--count==0) released(); }
> long refCount() { return count; }
>
> protected:
> virtual ~RefCounted() {};
> // Allow subclasses to over-ride behavior when refcount reaches 0.
> virtual void released() const { delete this; }
> };
>
> // intrusive_ptr support.
> inline void intrusive_ptr_add_ref(const RefCounted* p) { p->addRef(); }
> inline void intrusive_ptr_release(const RefCounted* p) { p->release(); }
>
>
> class DataTokenImpl : public RefCounted
> {
> public:
> DataTokenImpl() {}
> ~DataTokenImpl() {}
> };
>
> void enqueue_data_record(const DataTokenImpl* dtokp) {
> // dtokp->release();
> throw 20;
> }
>
> int local()
> {
> try {
> boost::intrusive_ptr<DataTokenImpl> dtokp(new DataTokenImpl);
> dtokp->addRef();
> enqueue_data_record(dtokp.get());
> }
> catch (int e) {
> std::cout << "local: caught exception " << e << std::endl;
> throw;
> }
> return 0;
> }
>
> int main(void)
> {
> try {
> local();
> }
> catch (int e) {
> std::cout << "main: caught exception " << e << std::endl;
> }
> return 0;
> }
>
>
> Diffs
> -----
>
> /trunk/qpid/cpp/src/qpid/legacystore/JournalImpl.h 1528730
> /trunk/qpid/cpp/src/qpid/legacystore/JournalImpl.cpp 1528730
>
> Diff: https://reviews.apache.org/r/14555/diff/
>
>
> Testing
> -------
>
> Valgrind used for reproducer returned no memory related problems, automated
> tests passed.
>
>
> Thanks,
>
> Pavel Moravec
>
>
