Destroying set of smart pointers segfaults

[Matthias Kaeppler]

Hi,

I thought it'd be a better idea to start a new thread dealing directly
with my problem. Okay, here's again what's happening:
I'm storing boost::shared_p trS in an std::set. I supposed that on
program exit, when the set's dtor is called, the shared_ptrS would clean
up after themselves appropriately. What actually happens though is that
the program segfaults when the set is destructed.

I noticed that erasing the smart pointers from the std::set does not
work as expected. IIRC, std::set finds its elements by checking for
/equivalance/, right? That means, it would check for !less && !greater
right?
So, does that work with boost::shared_p tr at all? I know it features an
operator==, but that would only work if the container would check for
equality, which is not the case.

But then, even if my observation would be correct, I can't explain why
destroying the set would cause the program to segfault.

Any ideas?

Thanks,
Matthias

[Matthias Kaeppler]

Here are the relevant parts of my program code:

class Task {/* ... */}; // the definition is not relevant

// this class stores the set in question
class TaskManager
{
public:
void add_task(Task*) ;
void remove_task(Tas k*);
// ...
private:
std::set< boost::shared_p tr<Task> > tasks_;
// ...
};

void TaskManager::ad d_task(Task *task)
{
tasks_.insert(b oost::shared_pt r<Task>(task)) ;
// ...
task->start();
}

void TaskManager::re move_task(Task *task)
{
task->stop();
// ...
tasks_.erase(bo ost::shared_ptr <Task>(task)) ; // this does in fact
NOT erase anything
}

Tasks are added by calling TaskManager::ad d_task():

// in some other class:
taskmanager.add _task(new Task(...));

Hope that helps.
- Matthias

[Max M.]

Matthias Kaeppler wrote:

tasks_.erase(bo ost::shared_ptr <Task>(task)) ;*//*this*does*in*f act

You can't legally initialize a new shared_ptr with a given pointer, if
there are other shared_ptr's around pointing to the same object. You
probably could, if an intrusive reference counting technique were used,
which is not the case.

You should probably handle every 'task' object using a shared_ptr as it
gets instantiated, and change TaskManager's interface so that it accepts
shared_ptr arguments.

Max

[David Hilsee]

"Matthias Kaeppler" <no*****@finite state.org> wrote in message
news:de******** *****@news.t-online.com...
[...]

tasks_.erase(bo ost::shared_ptr <Task>(task)) ; // this does in fact
NOT erase anything
}

[...]

As it shouldn't. From the docs:

------
under the equivalence relation defined by operator<, !(a < b) && !(b < a),
two shared_ptr instances are equivalent if and only if they share ownership
or are both empty.
------

The shared_ptr in the set container doesn't share ownership with the
shared_ptr that you just created (they are unaware of each other's
existence), so they are not considered equivalent.

--
David Hilsee

[Pete Becker]

Matthias Kaeppler wrote:

Here are the relevant parts of my program code:

class Task {/* ... */}; // the definition is not relevant

// this class stores the set in question
class TaskManager
{
public:
void add_task(Task*) ;
void remove_task(Tas k*);
// ...
private:
std::set< boost::shared_p tr<Task> > tasks_;
// ...
};

This is getting you in trouble. You can't mix raw pointers and
shared_ptr. If you're going to use a shared_ptr to control a resource
you must initialize a shared_ptr object with the pointer to the resource
and then abandon the pointer. (It's not quite that drastic, but it's a
good rule of thumb when you're getting started):

shared_ptr<Task > sp(new Task);

And now your TaskManager's interface should traffic in shared_ptr<Task >s
rather than Task*'s.
--

Pete Becker
Dinkumware, Ltd. (http://www.dinkumware.com)

[Matthias Kaeppler]

Okay, thanks for all your input.

One more question:
When passing around a shared_ptr, is it better practice to do it by
reference or by value?

e.g.:
void foo(const shared_ptr<Bar> & ptr);
or
void foo(shared_ptr< Bar> ptr);

I mean, constructing a copy is certainly not that expensive I guess, but
if it's reference counted, it would imply a useless increment and
decrement of the reference counter right?
Right now, I pass it by reference to const, just to be safe, although
this looks kind of weird for a "pointer" (I usually try to keep my code
intuitive, and this would imply to pass it by value just like a raw
pointer).

Regards,
Matthias