Best way to handle ownership

[Howard]

Hi,

I recently had a problem where I decided to store objects in a vector.
(Previously, I had always stored pointers in vectors). Well, naturally,
when storing an object in a vector, using push_back, the object I had in
hand was getting copied (twice, in fact). That led to a problem, in that my
object contained a "handle" to another object, and when the object being
pushed went out of scope and was destroyed, the referenced object was also
destroyed, making the copy inside the vector refer to a now non-existent
object. Changing my vector to store pointers instead of objects resolved
that problem. But I'm wondering if that's the "best" way to handle the
problem.

I see a few options.

One was to create a copy constructor for my object, which created a new
object and a new sub-object as well. But that had two problems. One, it
meant I was using extra resources and time making that extra sub-object that
I didn't really need. Two, it meant that to get the "handle" that was
stored in the object I pushed into the vector (which I wanted to return from
the function that was doing the push_back), I had to get it by extracting
the object back out of the vector and querying it, which seemed a waste.

Another idea was to make my copy-constructor take ownership of the
handle from the passed object. That meant that the copy constructor had to
change the contents of the object passed to it (to set the handle to NULL),
but... I had declared the parameter as a const reference! I didn't feel
that changing that from a const reference was a great idea (although I could
be wrong...it's just that whenever I make a copy constructor, I always make
the parameter a const reference).

And lastly, I could store pointers in the vector (and write code to
iterate of the vector to destroy the objects when I was done with the list).
That's what I chose to do eventually. But I avoided it at first because I
have no need for polymorphism, so storing pointers wasn't really needed,
except to resolve this ownership issue.

Which of the above methods might be considered "better"? (Or is there
another paradigm for this kind of work with containers? Perhaps using some
kind of reference-counted pointer instead of the bare "handle" inside my
object?)

Thanks,
Howard

 

[Guest]

I recently had a problem where I decided to store objects in a vector.
(Previously, I had always stored pointers in vectors). Well, naturally,
when storing an object in a vector, using push_back, the object I had in
hand was getting copied (twice, in fact). That led to a problem, in that
my object contained a "handle" to another object, and when the object
being pushed went out of scope and was destroyed, the referenced object
was also destroyed, making the copy inside the vector refer to a now
non-existent object.

Did you not observe the rule of three? You obviously wrote a destructor,
but did not define the copy constructor or at least declared it privately.

Changing my vector to store pointers instead of objects resolved that
problem. But I'm wondering if that's the "best" way to handle the
problem.

If copying your objects is not viable for any reason, you should disallow
copying. In that case, your only option is to use pointers, which bring
their ownership problems, hence your subject line.

For that reason, the best thing to do would be to store smart pointers in
the vector:

#include <boost/shared_ptr.hpp>

typedef boost::shared_ptr<MyClass> MyClassPtr;

#include <vector>

I see a few options.

One was to create a copy constructor for my object, which created a new
object and a new sub-object as well. But that had two problems. One, it
meant I was using extra resources and time making that extra sub-object
that I didn't really need. Two, it meant that to get the "handle" that
was stored in the object I pushed into the vector (which I wanted to
return from the function that was doing the push_back), I had to get it by
extracting the object back out of the vector and querying it, which seemed
a waste.

Another idea was to make my copy-constructor take ownership of the
handle from the passed object.

Not a good idea. That is precisely the reason why std::auto_ptr cannot be
used with standard containers.

Standard containers require that the objects are copy constructible and that
the copies are equivalent.

That meant that the copy constructor had to change the contents of the
object passed to it (to set the handle to NULL), but...

Don't do it

I had declared the parameter as a const reference! I didn't feel that
changing that from a const reference was a great idea (although I could be
wrong...it's just that whenever I make a copy constructor, I always make
the parameter a const reference).

Could you use a reference counted pointer (e.g. boost::shared_ptr) for the
handle? That would make the copies of your objects "share the handle"
instead of "own the handle."

And lastly, I could store pointers in the vector (and write code to
iterate of the vector to destroy the objects when I was done with the
list).

Calling delete explicitly makes your code exception-unsafe. Use smart
pointers in the vector.

That's what I chose to do eventually. But I avoided it at first because I
have no need for polymorphism, so storing pointers wasn't really needed,
except to resolve this ownership issue.

Pointers are not only for polymorphism. Dynamic objects allow us extend
object lifetimes: they are not automatic objects, meaning that they don't
get destroyed automatically.

Which of the above methods might be considered "better"? (Or is there
another paradigm for this kind of work with containers? Perhaps using
some kind of reference-counted pointer instead of the bare "handle" inside
my object?)

Either that, or use reference counted pointers in the vector.

Ali
--
Plug: ACCU's Silicon Valley Chapter meets on second Tuesdays. The meetings
are open to public and free of charge. Please come tonight for a talk on
Ada:

http://accu-usa.org/index.html

 

[Peter Jansson]

Hi,

I recently had a problem where I decided to store objects in a vector.
(Previously, I had always stored pointers in vectors). Well, naturally,
when storing an object in a vector, using push_back, the object I had in
hand was getting copied (twice, in fact). That led to a problem, in that my
object contained a "handle" to another object, and when the object being
pushed went out of scope and was destroyed, the referenced object was also
destroyed, making the copy inside the vector refer to a now non-existent
object. Changing my vector to store pointers instead of objects resolved
that problem. But I'm wondering if that's the "best" way to handle the
problem.

I see a few options.

One was to create a copy constructor for my object, which created a new
object and a new sub-object as well. But that had two problems. One, it
meant I was using extra resources and time making that extra sub-object that
I didn't really need. Two, it meant that to get the "handle" that was
stored in the object I pushed into the vector (which I wanted to return from
the function that was doing the push_back), I had to get it by extracting
the object back out of the vector and querying it, which seemed a waste.

Another idea was to make my copy-constructor take ownership of the
handle from the passed object. That meant that the copy constructor had to
change the contents of the object passed to it (to set the handle to NULL),
but... I had declared the parameter as a const reference! I didn't feel
that changing that from a const reference was a great idea (although I could
be wrong...it's just that whenever I make a copy constructor, I always make
the parameter a const reference).

And lastly, I could store pointers in the vector (and write code to
iterate of the vector to destroy the objects when I was done with the list).
That's what I chose to do eventually. But I avoided it at first because I
have no need for polymorphism, so storing pointers wasn't really needed,
except to resolve this ownership issue.

Which of the above methods might be considered "better"? (Or is there
another paradigm for this kind of work with containers? Perhaps using some
kind of reference-counted pointer instead of the bare "handle" inside my
object?)

Thanks,
Howard

Hello Howard,
Could you please provide a sample of the code you selected to use?
Regards,
Peter Jansson

 

[Howard]

Hello Howard,
Could you please provide a sample of the code you selected to use?
Regards,
Peter Jansson

Sure:

HFONT AFonts::FontByNameAndSize( const char* name, short size )
{
HFONT hFont = 0;
bool found = false;
// see if font already created
for (AFontIterator iter = fFonts.begin(); iter != fFonts.end(); ++iter )
{
AFont* pFont = (AFont*)&(*iter);
if ((!strcmp(pFont->fName,name)) &&
(pFont->fSize == size))
{
hFont = pFont->fHFont;
found = true;
}
}
// if not, create it
if (!found)
{
AFont* pFont = new AFont( name, size );
hFont = pFont->fHFont;
fFonts.push_back( pFont );
}
return hFont;
}

previously, that last part was like this:

AFont tempFont( name, size );
hFont = tempFont.fHFont;
fFonts.push_back( tempFont );

(The fHFont is the "handle", a Windows Font resource handle, which I create
in the constructor of an AFont object.)

-Howard

 

[Greg]

Hi,

I recently had a problem where I decided to store objects in a vector.
(Previously, I had always stored pointers in vectors). Well, naturally,
when storing an object in a vector, using push_back, the object I had in
hand was getting copied (twice, in fact). That led to a problem, in that my
object contained a "handle" to another object, and when the object being
pushed went out of scope and was destroyed, the referenced object was also
destroyed, making the copy inside the vector refer to a now non-existent
object. Changing my vector to store pointers instead of objects resolved
that problem. But I'm wondering if that's the "best" way to handle the
problem.

Especially for large objects or those with complicated copy semantics,
storage in a vector by pointer will be far more efficient than storage
by value. The disadvantage of storing pointers though is that the
vector will not manage the lifetimes of the objects referenced. As a
consequence, the client is tasked with the chore of deallocating each
object when its pointer is removed from the vector.

I see a few options.

One was to create a copy constructor for my object, which created a new
object and a new sub-object as well. But that had two problems. One, it
meant I was using extra resources and time making that extra sub-object that
I didn't really need. Two, it meant that to get the "handle" that was
stored in the object I pushed into the vector (which I wanted to return from
the function that was doing the push_back), I had to get it by extracting
the object back out of the vector and querying it, which seemed a waste.

Copying the data along with the object is not only inefficient, it can
also lead to consistency issues. If there is more than one copy of a
data set, which one is "authoritative"? Or is each copy even a copy and
not just its own unique set of data?

Another idea was to make my copy-constructor take ownership of the
handle from the passed object. That meant that the copy constructor had to
change the contents of the object passed to it (to set the handle to NULL),
but... I had declared the parameter as a const reference! I didn't feel
that changing that from a const reference was a great idea (although I could
be wrong...it's just that whenever I make a copy constructor, I always make
the parameter a const reference).

I would call this technique the "hot potato" approach toward resource
ownership. The last object to take possession of the resource is
responsible for freeing it. And while this approach can work, it is
constraining. But as long as the application's copy operations
involving the resource are both limited and not very complicated, it
can be a reasonable choice.

And lastly, I could store pointers in the vector (and write code to
iterate of the vector to destroy the objects when I was done with the list).
That's what I chose to do eventually. But I avoided it at first because I
have no need for polymorphism, so storing pointers wasn't really needed,
except to resolve this ownership issue.

As noted above, storing raw pointers can make for very efficient
vectors, but also require client code to manage the lifetimes of the
referenced objects.

Which of the above methods might be considered "better"? (Or is there
another paradigm for this kind of work with containers? Perhaps using some
kind of reference-counted pointer instead of the bare "handle" inside my
object?)

From what I can tell, storing a reference-counted "smart" pointer in

the vector would likely be the optimal approach. The smart pointer
would not reference the object's internal data handle (which I gather
is not shared), but rather it would be a used as a pointer to the
object itself.

Storing a smart pointer is nearly as efficient as storing a raw pointer
- but unlike a raw pointer, a smart pointer will automatically manage
the referenced object's lifetime. Because it is an object masquerading
as a pointer, a smart pointer can use its life time to manage the life
time of the referenced object, In short, when a smart pointer is
removed from the vector, the object it points to will automatically be
freed (assuming it has no other outstanding references).

For a smart pointer implementation look either for std::tr1::smart_ptr
or the boost version on which it is based.

Greg

 

[Howard]

Did you not observe the rule of three? You obviously wrote a
destructor, but did not define the copy constructor or at least declared
it privately.

Originally, I had the copy constructor copy the handle, but that led to the
problem of the temporary beign destroyed also destoying the handle I wanted
to keep. So I changed it create a new resource and handle for that
resource.

But that didn't resolve all the issues, because I wanted to return the
internal handle, and the copy made when calling push_back caused my copy
constructor to create a new handle, which was now not the same one I was
returning.

Rather than try to get the actual handle from the object in the vector, (and
because creating a new handle in the first place seemed a waste of
resources), I decided to settle on storing pointers in the vector.

I had this:
{
AFont tempFont( name, size );
// this handle is what I want, but it gets destroyed along with the
temporary
hFont = tempFont.fHFont;
fFonts.push_back( tempFont );
}
...
return hFont;

Now I have this:
{
AFont* pFont = new AFont( name, size );
// now my handle is correct
hFont = pFont->fHFont;
fFonts.push_back( pFont );
}
...
return hFont;

If copying your objects is not viable for any reason, you should disallow
copying. In that case, your only option is to use pointers, which bring
their ownership problems, hence your subject line.

For that reason, the best thing to do would be to store smart pointers in
the vector:

#include <boost/shared_ptr.hpp>

I don't have any of the boost stuff, and so far my boss is resistant to my
using their stuff, since it's not part of the standard C++ libraries. (I'll
work on that, though.)

typedef boost::shared_ptr<MyClass> MyClassPtr;

#include <vector>



Not a good idea. That is precisely the reason why std::auto_ptr cannot be
used with standard containers.

Standard containers require that the objects are copy constructible and
that the copies are equivalent.

Ah, ok.

Don't do it


Could you use a reference counted pointer (e.g. boost::shared_ptr) for the
handle? That would make the copies of your objects "share the handle"
instead of "own the handle."


Calling delete explicitly makes your code exception-unsafe. Use smart
pointers in the vector.


Pointers are not only for polymorphism. Dynamic objects allow us extend
object lifetimes: they are not automatic objects, meaning that they don't
get destroyed automatically.

Yeah, I know that, but given the choice between raw pointers and objects, I
chose the object at first, for the same reason: lifetime management.

Either that, or use reference counted pointers in the vector.

Yep, that would be a good option. One more reason why I'd like to get and
use the boost libraries (and get my boss to use them, too.)

Thanks,
Howard

 

[Howard]

For a smart pointer implementation look either for std::tr1::smart_ptr
or the boost version on which it is based.

Greg

I don't think tr1's available in VC++ 7.0 (Visual Studio 2002), which is
what I'm stuck with for now.

Thanks, though. You're probably right that that's the way to go.

-Howard

 

[Howard]

// see if font already created
for (AFontIterator iter = fFonts.begin(); iter != fFonts.end(); ++iter )
{
AFont* pFont = (AFont*)&(*iter);
if ((!strcmp(pFont->fName,name)) &&
(pFont->fSize == size))
{
hFont = pFont->fHFont;
found = true;
}
}

I just noticed that I should have a "break;" statement there. No sense
continuing the loop once it's found.

I could also drop the boolean and simply return the handle here, but I hate
having multiple return points in a function.

Probably even better would be to put that loop in a find() function of its
own,reducing this function to a simple if/else. Something like this:

return (HFONT hFont = FindFont(name,size)) ? hFont : NewFont(name,size);

where FindFont returns the handle if the font is in the vector (NULL
otherwise), and NewFont creates a new AFont object, pushes it onto the
vector, and returns its handle.

-Howard

 

[Guest]

I don't have any of the boost stuff, and so far my boss is resistant to my
using their stuff, since it's not part of the standard C++ libraries.
(I'll work on that, though.)

You really should work on convincing your boss to use smart pointers. Use
this snippet to show your boss the resource leaks in your application(s):

void foo()
{
Class * p = new Class(/* ... */);
code_that_may_throw_now_or_in_the_future(/* ... */);

// We may never get here
delete p;
}

Besides, Boost libraries are as best as you can get in any library:
peer-reviewed designs and implementations by C++ experts that are
extensively tested and used. Boost has been recognized as the non-official
extension to the C++ standard for years now. Especially the smart pointers
has been out for years with all the wrinkles ironed out.

In fact, they are so good that some of it is already approved to be part of
the standard. For example, shared_ptr is part of C++ TR1. Here is
an--unfortunately extremely slow--link on what Matt Austern covered in his
talk on TR1 at the last meeting of ACCU's Silicon Valley Chapter:

http://lafstern.org/matt/ACCU_tr1_talk.pdf

[It is still loading for me; his uplink must be very narrow Anyway, you
can find lots of information on TR1 on the net.]

What I am trying to say is that, shared_ptr should be considered as being
part of the C++ Standard already.

Ali
--
Plug: ACCU's Silicon Valley Chapter meets on second Tuesdays. The meetings
are open to public and free of charge. Please come tonight for a talk on
Ada:

http://accu-usa.org/index.html

 

[Howard]

I don't have any of the boost stuff, and so far my boss is resistant to
my using their stuff, since it's not part of the standard C++ libraries.
(I'll work on that, though.)

You really should work on convincing your boss to use smart pointers. Use
this snippet to show your boss the resource leaks in your application(s):

void foo()
{
Class * p = new Class(/* ... */);
code_that_may_throw_now_or_in_the_future(/* ... */);

// We may never get here
delete p;
}

Besides, Boost libraries are as best as you can get in any library:
peer-reviewed designs and implementations by C++ experts that are
extensively tested and used. Boost has been recognized as the non-official
extension to the C++ standard for years now. Especially the smart pointers
has been out for years with all the wrinkles ironed out.

In fact, they are so good that some of it is already approved to be part
of the standard. For example, shared_ptr is part of C++ TR1. Here is
an--unfortunately extremely slow--link on what Matt Austern covered in his
talk on TR1 at the last meeting of ACCU's Silicon Valley Chapter:

http://lafstern.org/matt/ACCU_tr1_talk.pdf

[It is still loading for me; his uplink must be very narrow Anyway, you
can find lots of information on TR1 on the net.]

What I am trying to say is that, shared_ptr should be considered as being
part of the C++ Standard already.

Ali

Thatnks, Ali. I agree 100%. I think the main reason the boss doesn't want
things not in our current compiler is so we have consistency between
ourselves and any potential contractors or temp programmers he hires.

But I can use all the ammo I can get in the fight!

Thanks again,
Howard

 

[Richard Herring]

Sure:

HFONT AFonts::FontByNameAndSize( const char* name, short size )
{
HFONT hFont = 0;
bool found = false;
// see if font already created
for (AFontIterator iter = fFonts.begin(); iter != fFonts.end(); ++iter )
{
AFont* pFont = (AFont*)&(*iter);

Unless your AFontIterator has very strange semantics, there's no need
for this. There's nothing (non-evil!) you'd want to do with the pointer
that the iterator can't do equally well, and having unnecessary
non-owning copies of pointers lying around is inviting trouble later on.

if ((!strcmp(pFont->fName,name)) &&

if ((!strcmp(iter->fName,name)) &&

 

[Howard]

Unless your AFontIterator has very strange semantics, there's no need
for this. There's nothing (non-evil!) you'd want to do with the pointer
that the iterator can't do equally well, and having unnecessary
non-owning copies of pointers lying around is inviting trouble later on.

if ((!strcmp(iter->fName,name)) &&

I had thought that it wasn't proper to use the iterator as if it were a
pointer, and had seen example code like this somewhere. If I can use the
iterator directly, as if it were actually an object pointer, then that's
great.

Thanks,
Howard

 

[Richard Herring]

I had thought that it wasn't proper to use the iterator as if it were a
pointer, and had seen example code like this somewhere.

What's improper is to assume that there is an implicit conversion

If I can use the
iterator directly, as if it were actually an object pointer, then that's
great.

You can't assume that an iterator *is* a pointer, but its pointer-*like*
behaviour (via operator*() and operator->() ) is guaranteed by the
standard at 24.1.

 

[Howard]

You can't assume that an iterator *is* a pointer, but its pointer-*like*
behaviour (via operator*() and operator->() ) is guaranteed by the
standard at 24.1.

Excellent. Thanks...
-Howard

 

[Mirek Fidler]

I recently had a problem where I decided to store objects in a vector.

(Previously, I had always stored pointers in vectors). Well, naturally,
when storing an object in a vector, using push_back, the object I had in
hand was getting copied (twice, in fact). That led to a problem, in that my
object contained a "handle" to another object, and when the object being
pushed went out of scope and was destroyed, the referenced object was also
destroyed, making the copy inside the vector refer to a now non-existent
object. Changing my vector to store pointers instead of objects resolved
that problem. But I'm wondering if that's the "best" way to handle the
problem.

Actually, this for me was the reason to avoid STL and try to develop
better variant. My solution goes like this:

Array<BigFoo> bigfoos;

BigFoo& n = bigfoos.Add();

// do whatever with newly created BigFoo.

Of course, this requires some shift in metodology - you have to provide
default constructor (but default constructor tend to be cheap) and
methods to setup object instead of using fat constructors, but overall I
believe that this is the "best" way how to handle this problem, of
course, as long as "best" is allowed to avoid "standard".

(see http://upp.sourceforge.net/srcdoc$NTL$en-us.html)

BTW, you can achieve the somewhat similar behaviour with STL as well

vector<BigFoo> bigfoos;
bigfoos.push_back(BigFoo());
BigFoo& n = bigfoos.back();

but it little but less effective and Array works even when BigFoo has no
copy constructor at all...

Mirek