Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

boost::python and set::erase -> weird behaviour

I'm trying to store objects in a std::set. Those objects are boost::shared_ptr<>, coming from the python environment. adding values to the set won't cause any troubles. But when I try to erase a value, even though I'm passing the very same reference, it won't work. Here is an example :

#include <set>
#include <iostream>

#include <boost/shared_ptr.hpp>
#include <boost/python.hpp>

using namespace std;
using namespace boost;
using namespace boost::python;

struct Bar
{
    Bar() {}
};

struct Foo
{
    set< shared_ptr<Bar> > v_set;
    shared_ptr<Bar> v_ptr;

    Foo() {}

    void add( shared_ptr<Bar> v_param ) {
    cout << "storing " << v_param << "in v_set and v_ptr" << endl;
    v_set.insert(v_param);
    v_ptr = v_param;

    }

    void del( shared_ptr<Bar> v_param ) {
    cout << "deleting " << v_param << endl;
    if (v_param == v_ptr) {
        cout << "v_param == v_ptr" << endl;
    } else {
        cout << "v_param != v_ptr" << endl;
    }

    cout << "erasing from v_set using v_param" << endl;
    if (v_set.erase(v_param) == 0) {
        cout << "didn't erase anything" << endl;
    } else {
        cout << "erased !" << endl;
    }

    cout << "erasing from v_set using v_ptr" << endl;
    if (v_set.erase(v_ptr) == 0) {
        cout << "didn't erase anything" << endl;
    } else {
        cout << "erased !" << endl;
    }
    }
};

BOOST_PYTHON_MODULE (test)
{
    class_< Foo, shared_ptr<Foo> >("Foo")
        .def("add",&Foo::add)
        .def("remove",&Foo::del);

    class_< Bar, shared_ptr<Bar> >("Bar");    
}

compiling :

%> gcc -pthread -fno-strict-aliasing -march=i686 -mtune=generic -O2 -pipe -DNDEBUG -march=i686 -mtune=generic -O2 -pipe -fPIC -I/usr/include/python2.7 -c test.cpp -o test.o

%> g++ -pthread -shared -Wl,--hash-style=gnu -Wl,--as-needed build/temp.linux-i686-2.7/test.o -L/usr/lib -lboost_python -lpython2.7 -o test.so

and now, a small python script :

from test import *

f = Foo()
b = Bar()

f.add(b)

f.remove(b)

Here is the result :

storing 0x8c8bc58in v_set and v_ptr
deleting 0x8c8bc58
v_param == v_ptr
erasing from v_set using v_param
didn't erase anything
erasing from v_set using v_ptr
erased !
  • I store 0x8e89c58 inside the set and outside, just in case
  • I'm passing the same reference to both calls (0x8e89c58)
  • just to make sure i check if v == val
  • I try to erase by using v -- it doesn't work
  • I try to erase by using val -- it works !

I'm completely lost there - can't see what is causing this. Any input ?

like image 980
girodt Avatar asked Nov 20 '11 17:11

girodt


1 Answers

I ran your example then added some assertions that I thought should hold in del():

assert(!(v_param < v_ptr));
assert(!(v_ptr < v_param));

One of them failed!

I dug into the implementation of operator< for boost::shared_ptr and found something strange: it compares the reference counts rather than the internal pointers! A little digging found a mailing list post about this issue with some helpful links to two C++ documents: N1590 which explains why people thought this was a good idea, and N2637 which explains why it wasn't.

It seems that the Boost people have not (yet?) adopted the N2637 recommendation, but C++11 has. So I built your test again using C++11 (g++ -std=c++0x), having removed using namespace boost; so as to use std::shared_ptr. This resulted in a horrible template-ridden error message which was solved by adding this at the top (easily derived from boost/smart_ptr/shared_ptr.hpp):

template<class T> inline T * get_pointer(std::shared_ptr<T> const & p)
{
    return p.get();
}

And it works!

If you can't use C++11, just implement your own custom comparator for your set which compares the pointers sanely:

template <typename T>
struct SmartComparator
{
    bool operator()(shared_ptr<T> const& lhs, shared_ptr<T> const& rhs) {
        return lhs.get() < rhs.get();
    }
};

Then this will work:

set< shared_ptr<Bar>, SmartComparator<Bar> > v_set;
like image 117
John Zwinck Avatar answered Sep 19 '22 23:09

John Zwinck