Delete raw pointer argument to boost::bind

Question

Lets say I have heap allocated A*, which I want to pass as argument to boost::bind. boost::bind is saved for later processing in some STL like container of boost::functions's.

I want to ensure A* will be destroyed at destruction of the STL container.

To demostrate:

A* pA = new A();

// some time later
container.push_back(boost::bind(&SomeClass::HandleA, this, pA);

// some time later
container is destroyed => pA is destroyed too

How can it be done?

EDIT

Maybe what I want is not that realistic.

I have raw pointer and function which receives the raw pointer. The call is delayed by means of boost::bind. At this point I want automatic memory management in case boost::bind want executed. I'm lazy, so I want to use "ready" smart-pointer solution.

std::auto_ptr looks like a good candidate, however ...

auto_ptr<A> pAutoA(pA);
container.push_back(boost::bind(&SomeClass::HandleA, this, pAutoA);

doesn't compile (see here)

auto_ptr<A> pAutoA(pA);
container.push_back(boost::bind(&SomeClass::HandleA, this, boost::ref(pAutoA));

pAutoA is destroyed, deleting underlying pA.

EDIT 02

In the mentioned container I will need to store misc "callbacks" with different arguments. Some of them are raw pointers to object. Since the code is old, I not always can change it.

Writing own wrapper for storing callbacks in container is last resort (while maybe the only one), hence bounty.

Frenchman

Answer 1

The idea of @pmjordan was already going in the right direction. You replied that you can't use shared_ptr, because you can't take ownership back from it once constructed. But that is not entirely correct: with shared_ptr's custom deleter mechanism, you can. This is how:

Assume these toy defintions for your A and f(A*):

struct A {
    ~A() { std::cout << "~A()" << std::endl; }
};

void f( A * a ) {
    std::cout << "in f(A*)" << std::endl;
    delete a;
}

1. Write a deleter that can be "switched off":

   struct opt_delete {
       bool m_delete;
       opt_delete() : m_delete( true ) {}
       template <typename T>
       void operator()( T * t ) {
           if ( m_delete ) delete t;
       }
   };
   

2. Then you can write a take() function that takes ownership of the shared_ptr payload again:

   template <typename T>
   T * take( const boost::shared_ptr<T> & sp ) {
       opt_delete * d = boost::get_deleter<opt_delete>( sp );
       assert( d );
       assert( d->m_delete == true );
       d->m_delete = false;
       return sp.get();
   }
   

   (this will leave the payload in the remaining shared_ptr instances, but for your case, that's ok, and the assert()s cover the cases when it's not).

3. Now you can manually wrap f(A*) like this:

   void f_sp( const boost::shared_ptr<A> & a ) {
       f( take( a ) );
   }
   

4. And finally, test the two scenarios:

   int main( int argc, char * argv[] ) {
   
       const boost::shared_ptr<A> a( new A, opt_delete() );
   
       const boost::function<void()> func =
           boost::bind( &f_sp, a );
   
       if ( argc >= 2 && *argv[1] == '1' ) // call 'func'
           func();
       else
           ; // don't
   
       return 0;
   }
   

Executing the test program with a 1 argument will print

in f(A*)
~A()

and without (or any other argument), it will print

~A()

You can extend the test harness to put func into a container first, but it'll still be safe. The only thing that isn't safe in the case is calling the func copies more than once (but then you'll trigger the second assertion in take()).

EDIT: Note that this mechanism isn't thread-safe. To make it thread-safe, you need to supply opt_delete with a mutex to synchronise operator() with take().

Hummer

+1: I like this answer. I suspected that a good solution should use custom deleters, and thought in this direction; what I forgot is that deleter can be a functor, and so each object can have its own flag. – Jaclin

Answer 2

I assume you mean you have some function, let's call it f() which takes an A*, which you then proxy with boost::bind? Can you change this function to accept a Boost/TR1 shared_ptr<A> instead? Using a shared_ptr (or, less likely, a C++98 std::auto_ptr) should solve your lifecycle problem.

Alternatively, if you can't change f itself, you could create a wrapper which accepts a shared_ptr<A>, pulls out the raw pointer and calls f with it. If you find yourself writing a lot of these wrappers, you may be able to create a template for generating them, assuming the function signatures are similar.

Prole

I can change f(), but can not change A* to shared_ptr<A*>. I tried to change A* to auto_ptr<A>, but boost:bind() and auto_ptr are not good friends lists.boost.org/Archives/boost/2002/10/38652.php - I need to do boost::bind(boost::ref(auto_ptr_a)), but I can't ensure auto_ptr_a will outlive boost:bind – Frenchman

I generally don't recommend using auto_ptr<> except for unusual circumstances. What's the problem with using shared_ptr<A>, exactly? – Prole

f() is very nested function, part of legacy code. It handles A* pointer memory manually. Since there's no way to release() shared_ptr<A>, I can not use it. In contrast, auto_ptr<A> I can release right in the beginning of f(). – Frenchman

What do you mean by there's no way to release a shared_ptr? Use boost::shared_ptr pA(new A()); to allocate memory on the HEAP and then pA.reset() to release it. – Bozen

shared_ptr<A>.reset() will delete A*, since it's the only reference to the A*. I rather want release() it, but shared_ptr doesn't offer it. – Frenchman

I'm confused. If f() frees the memory explicitly, I don't see why you need automatic memory management at all. Won't binding the raw pointer directly work fine then? – Prole

You understand correctly. I want automatically free A* which hold by boost::function in STL container. – Frenchman

@dimba, If that's really what you want, just pass the pointer to a shared_ptr<A> and neither reset nor release it. It will then be deleted when the corresponding boost::function is destructed. – Urticaceous

I start thinking I'm asking for something not realistic or most probably I don't know to explain myself well :) In the start A* should be owned by boost::bind, so in case boost::function is not executed at all (i.e. it's still in STL container) it should be removed by boost::function. In case boost::function executes f(), ownership for A* should be passed to f(). f() is really deeply nested method, which I can not and don't want to touch. So I need a way to receive raw A* from boost::function and leave it to be managed by f() (e.g. auto_ptr<A>.get(), followed by auto_ptr<A>/release()). – Frenchman

To be honest, this actually sounds like a case where auto_ptr<> is the right solution. Test through the two cases with a dummy class that prints from its destructor to make sure it does what it's supposed to. If it doesn't work, you'll have to wrap it manually. – Prole

Answer 3

NB! This is UGLY!

Have just scrateched some proof of concept. Well, it does what requested, as far as I can see - but this stuff relies on const_cast assumption. If you decide to use something like that in your program, be ready to double check all copy constructions happening in your program all the time, and using valgrind to verify nothing is leaked/corrupted.

Trick is in defining you own wrapper class, that ignores const qualifiers and allows auto_ptr ownership transfer from const referenced auto_ptr. This can get crazy if you ll try, for example, copy vector itself.

So be sure to read carefuly about vector copy semantics, auto_ptr ownership transfer semantics and, best of all - just use shared_ptr :)

#include <iostream>
#include <boost/bind.hpp>
#include <algorithm>
#include <vector>
#include <boost/function.hpp>

class parameter_data
{
    public:
    ~parameter_data()
    {
        std::cout << "~parameter_data()" << std::endl;
    }

    parameter_data()
    {
        std::cout << "parameter_data()" << std::endl;
    }
};

void f( parameter_data* data )
{
    std::cout << "Processing data..." << std::endl;
};


class storage_wrapper
{
    private:
        boost::function<void()> callable;
        std::auto_ptr<parameter_data> data;
    public:
        storage_wrapper( const storage_wrapper& copy ) 
        {
            callable = const_cast< storage_wrapper&>(copy).callable;
            data = const_cast< storage_wrapper&>(copy).data;
        }

        storage_wrapper( parameter_data *adata )
            : data( adata )
        {
            callable = boost::bind( &f, adata );
        }

        storage_wrapper& operator=( const storage_wrapper& copy)
        {
            callable = const_cast< storage_wrapper&>(copy).callable;
            data = const_cast< storage_wrapper&>(copy).data;
        }

        void operator()()
        {
            callable();
        }
};

int main()
{
    std::cout << "Start of program" << std::endl;
    {
        std::vector<storage_wrapper> container;
        for ( int i = 0; i < 100; i++ )
            container.push_back( storage_wrapper( new parameter_data() ) );
        for ( int i = 0; i < 100; i++ )
            container[i]();
    }
    std::cout << "End of program" << std::endl;
    return 0;
}

Jerrold

This is similar to what I had before - container was storing storage_wrapper*, while storage_wrapper d-tor was deleting parameter_data. I try to avoid such manual solution - I want to avoid writing storage_wrapper class – Frenchman

Please ignore my previous comment - I wasn't unable to edit it. This is similar to what I had before - storage_wrapper was holding parameter_data* and in its d-tor was deleting parameter_data*, while container was storing storage_wrapper*. At time of container destruction, code was iterating all elements and deleting them. Your solution has more robust memory management. Hoverer, I'm trying to do it even more and do not have storage_wrapper class at all. So code will remain only with essentially parts - container, parameter_data and callback in form of boost::bind – Frenchman

Well, I hardly can imagine any way to it without writing some small wrapper - stuff you want breaks some safety rules about automatic ownership manipulation. I doubt there can be any library solution that provides hacky way. My code has additional benefit that you can release ownership of auto_ptr before calling f() in operator() of wrapper - and that will allow f() to release memory itself without modifying f() at all. – Jerrold

Answer 4

It doesn't need to be very complex:

class MyContainer : public std::vector<boost::function<void ()> > {
public:
   void push_back(boost::function<void ()> f, A *pA) 
       { push_back(f); vec.push_back(pA); }
   ~MyContainer() 
       { int s=vec.size; for(int i=0;i<s;i++) delete vec[i]; }
private:
   std::vector<A*> vec;
};

It has one problem that you need to pass it to other functions via MyContainer & instead of std::vector reference, otherwise the original push_back can be called and it allows for cases where you can push_back without providing the A* pointer. Also it has no check for bind parameters to be the same A* object than pA. You can fix that by changing the push_back prototype:

template<class T>
void push_back(T *object, void (T::*fptr)(), A *pA) 
{
   push_back(boost::bind(fptr, object, pA)); vec.push_back(pA);
} 

Fatigued

hmm, one idea for fixing some problems with this solution is to make the inheritance private. But then it's all about wrapping solution. It still doesn't work with insert() functions etc, so this solution is slightly bad, depending on how you use std::vector's interface in your program. – Fatigued

see EDIT 02 - I can put callbacks to container with different types and number of arguments. Than I don't think proposed by you solution will fit. – Frenchman

oh, that's slightly more difficult as boost::function does not support it. You'll need something like class I { virtual void *data(int num)const=0; virtual std::string type(int num) const=0; }; and then use std::vector<I*> vec; and typeid(T).name(); and then implement it as described in the push_back solution. The implementation should have constructor with proper parameters. – Fatigued