Does push_back() do a copy?

[Angus]

I have a socket class CTestClientSocket which I am using to simulate
load testing.

I create multiple instances of the client like this:

for (int i = 0; i < 5; i++)
{
CTestClientSocket* pTemp = new CTestClientSocket(this, ip, port);
pTemp->Connect();
m_collClients.push_back(pTemp);
// delete pTemp;
}

Note I have commented out the delete pTemp

Then I send some data to a server like this:

// iterate through collection of clients
for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
(*it)->Send(byData);
}

This works if delete pTemp on the Connect is commented out. But I
thought that push_back would make a copy of pTemp. and so I could
then delete pTemp. But if I delete pTemp I get an memory access
problem in the (*it)->Send(byData);

And I have to cleanup at the end. Does push_back NOT do a copy? I am
confused, can someone explain.

Or is there a better way to do this sort of thing?

[Stefan Naewe]

On 6/28/2007 12:52 PM, Angus wrote:

I have a socket class CTestClientSocket which I am using to simulate
load testing.

I create multiple instances of the client like this:

for (int i = 0; i < 5; i++)
{
CTestClientSocket* pTemp = new CTestClientSocket(this, ip, port);
pTemp->Connect();
m_collClients.push_back(pTemp);
// delete pTemp;
}

Note I have commented out the delete pTemp

Then I send some data to a server like this:

// iterate through collection of clients
for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
(*it)->Send(byData);
}

This works if delete pTemp on the Connect is commented out. But I
thought that push_back would make a copy of pTemp.

And what happens if you copy a pointer ?

and so I could
then delete pTemp. But if I delete pTemp I get an memory access
problem in the (*it)->Send(byData);

Because you have a dangling pointer.

>
And I have to cleanup at the end. Does push_back NOT do a copy? I am
confused, can someone explain.

Or is there a better way to do this sort of thing?

Use boost::shared_ptr<for example.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

[Sumit Rajan]

Angus wrote:

I have a socket class CTestClientSocket which I am using to simulate
load testing.

I create multiple instances of the client like this:

for (int i = 0; i < 5; i++)
{
CTestClientSocket* pTemp = new CTestClientSocket(this, ip, port);
pTemp->Connect();
m_collClients.push_back(pTemp);
// delete pTemp;
}

When your "delete pTemp;" is uncommented, you're deleting the objects
you have new'd. So, what you have is a vector with pointers that do
point to objects that no longer exist.

Note I have commented out the delete pTemp

Then I send some data to a server like this:

// iterate through collection of clients
for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
(*it)->Send(byData);
}

And when you try to play around with pointers that point to non-existent
objects, you're triggering undefined behaviour.

>
This works if delete pTemp on the Connect is commented out. But I
thought that push_back would make a copy of pTemp. and so I could
then delete pTemp. But if I delete pTemp I get an memory access
problem in the (*it)->Send(byData);

And I have to cleanup at the end. Does push_back NOT do a copy? I am
confused, can someone explain.

Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.

Regards,
Sumit.

--
Sumit Rajan <su*********@gmail.com>

[Bobbin]

>
Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.

As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.

[Kai-Uwe Bux]

Angus wrote:

I have a socket class CTestClientSocket which I am using to simulate
load testing.

I create multiple instances of the client like this:

for (int i = 0; i < 5; i++)
{
CTestClientSocket* pTemp = new CTestClientSocket(this, ip, port);
pTemp->Connect();
m_collClients.push_back(pTemp);
// delete pTemp;
}

Note I have commented out the delete pTemp

Then I send some data to a server like this:

// iterate through collection of clients
for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
(*it)->Send(byData);
}

This works if delete pTemp on the Connect is commented out. But I
thought that push_back would make a copy of pTemp.

Yes.

and so I could then delete pTemp.

No: pTemp is a pointer and that is all that is copied. The pointee is _not_
copied. Thus, if you delete pTemp, you loose the pointee and your program
is in deep undefined waters.

But if I delete pTemp I get an memory access
problem in the (*it)->Send(byData);

Right.

And I have to cleanup at the end. Does push_back NOT do a copy? I am
confused, can someone explain.

With pointers, you need to distinguish between the pointer and the pointee.
Both are objects. When used in standard containers, the former is copied,
the later is not.
Best

Kai-Uwe Bux

[Stefan Naewe]

On 6/28/2007 1:30 PM, Bobbin wrote:

>Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.

As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.

Not!

The OP had this:

std::vector<CTestClientSocket*>

You can't put CTestClientSocket _object_ into that!.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

[Angus]

On 28 Jun, 12:45, Stefan Naewe <[e-mail]:

On 6/28/2007 1:30 PM, Bobbin wrote:

Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.

As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.

Not!

The OP had this:

std::vector<CTestClientSocket*>

You can't put CTestClientSocket _object_ into that!.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

This is someone elses code. I have just noticed this:

//-----------------------------------------------------------------------
// operator= NOT IMPLEMENTED
//-----------------------------------------------------------------------
const CTestClientSocket & operator=(
const CTestClientSocket & krobToAssign)
throw();

As there is no copy implemented then when I push_back() then the
object is not properly copied? Is this my problem? I implement the
copy consstructor and then I can uncomment the delete pTemp?

[Stefan Naewe]

On 6/28/2007 2:59 PM, Angus wrote:

On 28 Jun, 12:45, Stefan Naewe <[e-mail]:

>On 6/28/2007 1:30 PM, Bobbin wrote:

>>>Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.
As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.

Not!

The OP had this:

std::vector<CTestClientSocket*>

You can't put CTestClientSocket _object_ into that!.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

This is someone elses code. I have just noticed this:

//-----------------------------------------------------------------------
// operator= NOT IMPLEMENTED
//-----------------------------------------------------------------------
const CTestClientSocket & operator=(
const CTestClientSocket & krobToAssign)
throw();

As there is no copy implemented then when I push_back() then the
object is not properly copied? Is this my problem? I implement the
copy consstructor and then I can uncomment the delete pTemp?

You cannot push _objects_ into a vector of _pointer to objects_.
Please get a book that explains the difference between objects
ant pointers.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

[=?ISO-8859-1?Q?Erik_Wikstr=F6m?=]

On 2007-06-28 14:59, Angus wrote:

On 28 Jun, 12:45, Stefan Naewe <[e-mail]:

>On 6/28/2007 1:30 PM, Bobbin wrote:

>Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.

As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.

Not!

The OP had this:

std::vector<CTestClientSocket*>

You can't put CTestClientSocket _object_ into that!.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

This is someone elses code. I have just noticed this:

//-----------------------------------------------------------------------
// operator= NOT IMPLEMENTED
//-----------------------------------------------------------------------
const CTestClientSocket & operator=(
const CTestClientSocket & krobToAssign)
throw();

As there is no copy implemented then when I push_back() then the
object is not properly copied? Is this my problem? I implement the
copy consstructor and then I can uncomment the delete pTemp?

Probably not, there's a good reason why a socket should not be copyable
(meaning that it should have no copy-constructor or assignment
operator). The reason is that a socket is considered a resource, and
there can only be one instance of each socket (where socket ==
connection). I'm just guessing, but I think the socket class looks
something like this:

class Socket {
// stuff to keep track of a conenction
public:
Socket(string host, int port) {
// Connect to host
}
void send();
void receive();
~Socket() {
// Disconnect
}
};

So if you add an assignment operator and do something like

for (int i = 0; i < 5; i++)
{
Socket s(host, ip);
m_collClients.push_back(s);
}

What will happen? First you create a socket, then you add a copy of the
socket to the collection. Then the socket you created goes out of scop,
which means that the destructor is called, which in turn means that the
connection is broken. So the socket you then have in the collection
would no longer have a working connection.

The correct solution is to do like you did, use new to create the
sockets, add the pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets you go through the
collection and calls delete on each element in it.

--
Erik Wikström

[Angus]

On 28 Jun, 14:07, Stefan Naewe <[e-mail]:

On 6/28/2007 2:59 PM, Angus wrote:

On 28 Jun, 12:45, Stefan Naewe <[e-mail]:

On 6/28/2007 1:30 PM, Bobbin wrote:

>>Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.
As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.
Not!

The OP had this:

std::vector<CTestClientSocket*>

You can't put CTestClientSocket _object_ into that!.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

This is someone elses code. I have just noticed this:

//-----------------------------------------------------------------------
// operator= NOT IMPLEMENTED
//-----------------------------------------------------------------------
const CTestClientSocket & operator=(
const CTestClientSocket & krobToAssign)
throw();

As there is no copy implemented then when I push_back() then the
object is not properly copied? Is this my problem? I implement the
copy consstructor and then I can uncomment the delete pTemp?

You cannot push _objects_ into a vector of _pointer to objects_.
Please get a book that explains the difference between objects
ant pointers.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html- Hide quoted text -

- Show quoted text -

What I want to store is a pointer to an object. The object being a
CTestClientSocket. Why can you not push a CTestClientSocket* into a
vector?

[Kai-Uwe Bux]

Angus wrote:

On 28 Jun, 14:07, Stefan Naewe <[e-mail]:

>On 6/28/2007 2:59 PM, Angus wrote:

On 28 Jun, 12:45, Stefan Naewe <[e-mail]:
On 6/28/2007 1:30 PM, Bobbin wrote:

>>>Yes, push_back, in your case, pushes a copy of the *pointer*. And
then
you destroy the object it points to.
As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.
Not!

>The OP had this:

> std::vector<CTestClientSocket*>

>You can't put CTestClientSocket _object_ into that!.

>S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

This is someone elses code. I have just noticed this:

//-----------------------------------------------------------------------

// operator= NOT IMPLEMENTED

//-----------------------------------------------------------------------

const CTestClientSocket & operator=(
const CTestClientSocket & krobToAssign)
throw();

As there is no copy implemented then when I push_back() then the
object is not properly copied? Is this my problem? I implement the
copy consstructor and then I can uncomment the delete pTemp?

You cannot push _objects_ into a vector of _pointer to objects_.
Please get a book that explains the difference between objects
ant pointers.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html-
Hide quoted text -

- Show quoted text -

What I want to store is a pointer to an object. The object being a
CTestClientSocket. Why can you not push a CTestClientSocket* into a
vector?

You can. More precisely, you can push_back object of type CTestClientSocket*
into a std::vector< CTestClientSocket* >. Nothing in the message you quoted
says you cannot.

What you have to understand is that the vector will only see and care about
the CTestClientSocket* and not about the CTestClientSocket object pointed
to. Only the former is copied and stored in the vector. The latter (the
pointee) will cease to exist once you call delete on the pointer. This will
be troublesome if you keep dereferencing pointers to the dead body of the
former object.

When working with pointers managing the lifetime of the objects they point
to is your responsibility.
Best

Kai-Uwe Bux

[Angus]

On 28 Jun, 16:11, Erik Wikström <[e-mail]:

On 2007-06-28 14:59, Angus wrote:

On 28 Jun, 12:45, Stefan Naewe <[e-mail]:

On 6/28/2007 1:30 PM, Bobbin wrote:

Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.

As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.

Not!

The OP had this:

std::vector<CTestClientSocket*>

You can't put CTestClientSocket _object_ into that!.

S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

This is someone elses code. I have just noticed this:

//-----------------------------------------------------------------------
// operator= NOT IMPLEMENTED
//-----------------------------------------------------------------------
const CTestClientSocket & operator=(
const CTestClientSocket & krobToAssign)
throw();

As there is no copy implemented then when I push_back() then the
object is not properly copied? Is this my problem? I implement the
copy consstructor and then I can uncomment the delete pTemp?

Probably not, there's a good reason why a socket should not be copyable
(meaning that it should have no copy-constructor or assignment
operator). The reason is that a socket is considered a resource, and
there can only be one instance of each socket (where socket ==
connection). I'm just guessing, but I think the socket class looks
something like this:

class Socket {
// stuff to keep track of a conenction
public:
Socket(string host, int port) {
// Connect to host
}
void send();
void receive();
~Socket() {
// Disconnect
}

};

So if you add an assignment operator and do something like

for (int i = 0; i < 5; i++)
{
Socket s(host, ip);
m_collClients.push_back(s);

}

What will happen? First you create a socket, then you add a copy of the
socket to the collection. Then the socket you created goes out of scop,
which means that the destructor is called, which in turn means that the
connection is broken. So the socket you then have in the collection
would no longer have a working connection.

The correct solution is to do like you did, use new to create the
sockets, add the pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets you go through the
collection and calls delete on each element in it.

--
Erik Wikström- Hide quoted text -

- Show quoted text -

Thank you.

So to delete I do this:

for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
delete *it;
}
m_collClients.erase(m_collClients.begin(), m_collClients.end());

Does that look right to you?

[=?ISO-8859-1?Q?Erik_Wikstr=F6m?=]

On 2007-06-28 18:20, Angus wrote:

On 28 Jun, 16:11, Erik Wikström <[e-mail]:

>On 2007-06-28 14:59, Angus wrote:

On 28 Jun, 12:45, Stefan Naewe <[e-mail]:
On 6/28/2007 1:30 PM, Bobbin wrote:

>Yes, push_back, in your case, pushes a copy of the *pointer*. And then
you destroy the object it points to.

As Sumit Rajan said, you just make a copy of the pointer, when you
write
push_back(*pTemp), it will works the way you expect.

>Not!

>The OP had this:

> std::vector<CTestClientSocket*>

>You can't put CTestClientSocket _object_ into that!.

>S.
--
Stefan Naewe stefan dot naewe at atlas-elektronik dot com
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html

This is someone elses code. I have just noticed this:

//-----------------------------------------------------------------------
// operator= NOT IMPLEMENTED
//-----------------------------------------------------------------------
const CTestClientSocket & operator=(
const CTestClientSocket & krobToAssign)
throw();

As there is no copy implemented then when I push_back() then the
object is not properly copied? Is this my problem? I implement the
copy consstructor and then I can uncomment the delete pTemp?

Probably not, there's a good reason why a socket should not be copyable
(meaning that it should have no copy-constructor or assignment
operator). The reason is that a socket is considered a resource, and
there can only be one instance of each socket (where socket ==
connection). I'm just guessing, but I think the socket class looks
something like this:

class Socket {
// stuff to keep track of a conenction
public:
Socket(string host, int port) {
// Connect to host
}
void send();
void receive();
~Socket() {
// Disconnect
}

};

So if you add an assignment operator and do something like

for (int i = 0; i < 5; i++)
{
Socket s(host, ip);
m_collClients.push_back(s);

}

What will happen? First you create a socket, then you add a copy of the
socket to the collection. Then the socket you created goes out of scop,
which means that the destructor is called, which in turn means that the
connection is broken. So the socket you then have in the collection
would no longer have a working connection.

The correct solution is to do like you did, use new to create the
sockets, add the pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets you go through the
collection and calls delete on each element in it.

--
Erik Wikström- Hide quoted text -

- Show quoted text -

Thank you.

So to delete I do this:

for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
delete *it;
}
m_collClients.erase(m_collClients.begin(), m_collClients.end());

Does that look right to you?

Yes

--
Erik Wikström

[Gavin Deane]

On 28 Jun, 17:20, Angus <[e-mail]:

On 28 Jun, 16:11, Erik Wikström <[e-mail]:

The correct solution is to do like you did, use new to create the
sockets,

within an appropriate RAII object (e.g. a smart pointer),

add the

objects that own the

pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets

.... the memory management is already fully solved and has become a non-
issue.

So to delete I do this:

for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
delete *it;}

m_collClients.erase(m_collClients.begin(), m_collClients.end());

That's what Erik suggested. However, it's not a very good solution.
Any time you have to write new or delete anywhere except in a class
designed to manage memory should ring big alarm bells.

Does that look right to you?

No. You're not using RAII. Someone has already suggested using a smart
pointer (boost::shared_ptr was the example). Do that - give the
responsibility for memory management to an object designed for the
purpose. You have much more important things to worry about.

Gavin Deane

[Marcus Kwok]

Erik Wikström <Er***********@telia.comwrote:

On 2007-06-28 18:20, Angus wrote:

>So to delete I do this:

for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
delete *it;
}
m_collClients.erase(m_collClients.begin(), m_collClients.end());

Does that look right to you?

Yes

Actually, formally it is undefined behavior to have deleted pointers in
a container; see:

http://groups.google.com/group/comp....b66e2112186cc4

However, in practice, it should work. But I guess if you wanted to be
strict about it, you could do something like in the referenced post, or
something like:

while (!m_collClients.empty()) {
CTestClientSocket* current = m_collClients.back();
m_collClients.pop_back();
delete current;
}

--
Marcus Kwok
Replace 'invalid' with 'net' to reply

[James Kanze]

On Jun 28, 8:37 pm, Gavin Deane <[e-mail]:

On 28 Jun, 17:20, Angus <[e-mail]:

On 28 Jun, 16:11, Erik Wikström <[e-mail]:

The correct solution is to do like you did, use new to create the
sockets,

within an appropriate RAII object (e.g. a smart pointer),

It's hard to imagine one with the appropriate semantics for a
socket. This is exactly the sort of thing where one wants
explicit destruction.

>add the objects that own the
>pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets

... the memory management is already fully solved and has become a non-
issue.

Memory management is best solved by using the Boehm collector.
The problem here isn't just memory management, it's lifetime
management. When does the connection exist? And it's something
that needs to be explicitly handled.

So to delete I do this:

for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
delete *it;}

m_collClients.erase(m_collClients.begin(), m_collClients.end());

That's what Erik suggested. However, it's not a very good solution.

It depends. I can see cases where it would be appropriate, e.g.
program shutdown. Most of the time, however, I imagine that the
socket will be deleted and removed from the container
explicitly, in reaction to some external event.

Any time you have to write new or delete anywhere except in a class
designed to manage memory should ring big alarm bells.

Anytime you start thinking that smart pointers are going to
solve all your object lifetime problems, a big alarm bell should
ring. Memory management is the role of the garbage collector
(you do use the Boehm collector, don't you). Object lifetime is
a design issue, and until you have defined what you want, you
can't decide the correct solution to manage it.

Does that look right to you?

No. You're not using RAII. Someone has already suggested using a smart
pointer (boost::shared_ptr was the example).

Which for something like Socket is almost certainly a poor
choice, which will likely end up with a lot of dead objects
hanging around.

Do that - give the
responsibility for memory management to an object designed for the
purpose. You have much more important things to worry about.

For memory management, certainly. But shared_ptr doesn't really
solve that very well; the Boehm collector is the simplest answer
for that. As for object lifetime management, there's no way you
can avoid it; it's an important design issue.

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[Gavin Deane]

On 29 Jun, 09:57, James Kanze <[e-mail]:

On Jun 28, 8:37 pm,GavinDeane<[e-mail]:

On 28 Jun, 17:20, Angus <[e-mail]:

On 28 Jun, 16:11, Erik Wikström <[e-mail]:
The correct solution is to do like you did, use new to create the
sockets,

within an appropriate RAII object (e.g. a smart pointer),

It's hard to imagine one with the appropriate semantics for a
socket. This is exactly the sort of thing where one wants
explicit destruction.

I was only thinking about appropriate semantics for managing the
memory. If the object pointed to is responsible for a resource of its
own too (a socket connection) then it too needs to have been written
with RAII (or rather, the corollary of RAII, Destruction is Resource
Release, which is usually the real point of RAII anyway) in mind, but
that's an orthogonal point.

add the objects that own the
pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets

... the memory management is already fully solved and has become a non-
issue.

Memory management is best solved by using the Boehm collector.
The problem here isn't just memory management, it's lifetime
management. When does the connection exist? And it's something
that needs to be explicitly handled.

.... via the interface of the class that's responsible for a socket
connection.

So to delete I do this:
for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
delete *it;}
m_collClients.erase(m_collClients.begin(), m_collClients.end());

That's what Erik suggested. However, it's not a very good solution.

It depends. I can see cases where it would be appropriate, e.g.
program shutdown. Most of the time, however, I imagine that the
socket will be deleted and removed from the container
explicitly, in reaction to some external event.

I don't understand the need for the loop before the call to erase. The
objects in the container (smart pointers) do that for you.

Any time you have to write new or delete anywhere except in a class
designed to manage memory should ring big alarm bells.

Anytime you start thinking that smart pointers are going to
solve all your object lifetime problems, a big alarm bell should
ring.

I wasn't thinking about object lifetimes. I was thinking about
exception safety and robustness in the face of changes to the code.

Memory management is the role of the garbage collector
(you do use the Boehm collector, don't you). Object lifetime is
a design issue, and until you have defined what you want, you
can't decide the correct solution to manage it.

Does that look right to you?

No. You're not using RAII. Someone has already suggested using a smart
pointer (boost::shared_ptr was the example).

Which for something like Socket is almost certainly a poor
choice, which will likely end up with a lot of dead objects
hanging around.

What do you mean by "dead object", how would I end up with one hanging
around, and (if it's not obvious from the definition of "dead object")
why would that be bad?

Gavin Deane

[James Kanze]

On Jun 30, 2:24 pm, Gavin Deane <[e-mail]:

On 29 Jun, 09:57, James Kanze <[e-mail]:

On Jun 28, 8:37 pm,GavinDeane<[e-mail]:

On 28 Jun, 17:20, Angus <[e-mail]:
On 28 Jun, 16:11, Erik Wikström <[e-mail]:
The correct solution is to do like you did, use new to create the
sockets,
within an appropriate RAII object (e.g. a smart pointer),

It's hard to imagine one with the appropriate semantics for a
socket. This is exactly the sort of thing where one wants
explicit destruction.

I was only thinking about appropriate semantics for managing the
memory. If the object pointed to is responsible for a resource of its
own too (a socket connection) then it too needs to have been written
with RAII (or rather, the corollary of RAII, Destruction is Resource
Release, which is usually the real point of RAII anyway) in mind, but
that's an orthogonal point.

I don't think so. A socket has (or probably should have) a
specific lifetime; it's lifetime is not something that you can
somehow "automate", because it depends on external events.
This isn't necessarily true, however, and I can think of
scenarios, where a socket would have a lifetime which
corresponds to scope. But that isn't the case here, since the
poster is intentionally allocating them.

There is a second potential problem with regards to using RAII
with a socket: releasing the resource can fail, and you may have
to handle the error. In practice, this would mean that you
cannot release the resource in the destructor. Again, this
depends somewhat on design.

RAII, in its simplest form, works best for objects whose
lifetime depends on scope. Things like shared_ptr can be used
for memory managment, but aren't really applicable for objects
with an explicit lifetime. And of course, today, if you're
starting a new project, the only reasonable course of action is
to use the Boehm collector if you can. So shared_ptr becomes
really only pertinant to legacy applications which can't use the
Boehm collector. (I'm speaking here of the boost::shared_ptr
using the default deleter. There are some special scenarios
where it is still very, very useful with a user defined deleter
object.)

>add the objects that own the
>pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets
... the memory management is already fully solved and has become a non-
issue.

Memory management is best solved by using the Boehm collector.
The problem here isn't just memory management, it's lifetime
management. When does the connection exist? And it's something
that needs to be explicitly handled.

... via the interface of the class that's responsible for a socket
connection.

I was thinking at a higher level. When does the lifetime end?
As a result of what criteria.

In the servers I currently work on, for example, the socket
connection's lifetime ends when the client logs out; an external
event. This is typical of a lot of TCP servers, I think. In a
client, it probably depends on what the connection is being used
for; a service connection used to obtain a specific bit of
information, then closed, will probably obey scope; a more or
less permanent connection used within a library will have an
explicit lifetime, with client code calling a specific function
to terminate its lifetime. (Certainly, the class which is
responsible for the socket will not want to listen on a specific
button to know when to close the socket.)

So to delete I do this:
for (std::vector<CTestClientSocket*>::iterator it =
m_collClients.begin();
it != m_collClients.end(); it++)
{
delete *it;}
m_collClients.erase(m_collClients.begin(), m_collClients.end());
That's what Erik suggested. However, it's not a very good solution.

It depends. I can see cases where it would be appropriate, e.g.
program shutdown. Most of the time, however, I imagine that the
socket will be deleted and removed from the container
explicitly, in reaction to some external event.

I don't understand the need for the loop before the call to erase. The
objects in the container (smart pointers) do that for you.

The whole point is that it is probably bad design to use smart
pointers for such objects. Most of the time, I imagine that the
socket object itself, or a decorator, will be aware of the
vector, and remove the object from the vector in its destructor.
In such cases, the above loop will *NOT* work; the correct logic
is:

while ( ! m_collClients.empty() ) {
m_collClients.back().requestShutdown() ;
}

If the container is being managed separately, however, the above
loop is a valid solution. Again, however, mainly in the case of
a clean shutdown; most of the time, the socket objects will be
removed from the collection by whoever is managing their
lifetime, e.g. when the other end disconnects, or the user
requests closing a specific connection.

Any time you have to write new or delete anywhere except in a class
designed to manage memory should ring big alarm bells.

Anytime you start thinking that smart pointers are going to
solve all your object lifetime problems, a big alarm bell should
ring.

I wasn't thinking about object lifetimes. I was thinking about
exception safety and robustness in the face of changes to the code.

Any changes to code have to respect the design. If the object
lifetime should be deterministic, trying to use shared_ptr won't
change that, and is more error prone than handling the
conditions explicitly.

Memory management is the role of the garbage collector
(you do use the Boehm collector, don't you). Object lifetime is
a design issue, and until you have defined what you want, you
can't decide the correct solution to manage it.

Does that look right to you?
No. You're not using RAII. Someone has already suggested using a smart
pointer (boost::shared_ptr was the example).

Which for something like Socket is almost certainly a poor
choice, which will likely end up with a lot of dead objects
hanging around.

What do you mean by "dead object", how would I end up with one hanging
around, and (if it's not obvious from the definition of "dead object")
why would that be bad?

A dead object is one that still "exists", but shouldn't,
according to the program logic. It's potentially a problem with
all types of garbage collection, including boost::shared_ptr.
With normal garbage collection, however, you expect it; you
don't use the destructor to "invalidate" the object, but a
special member function, which marks the object as invalid. An
assertion check at the start of all functions then detects any
invalid use. With shared_ptr, there is a tendancy to count on
the destructor, which results in the object not being
invalidated when it should be, because someone still has a
pointer to it hanging around.

I've often felt that in the case of such objects with specific
lifetime, you really need some sort of "reverse garbage
collection"; something that would get rid of pointers to the
object whenever the object dies. I once had (and in fact still
have) a managed pointer which will null when the destructor of
the pointed to object is called. (This can be simulated with
boost::shared_ptr by having the object itself contain the only
shared_ptr, and all other objects use weak_ptr. Personally, I
find this a bit of obfuscation---setting the local shared_ptr to
null is a very unobvious way of writing "delete this".) In
practice, I've almost never used it, since it's usually
necessary to do more than just set the pointers to null, and
some other notification mechanism is required anyway.

--
James Kanze (Gabi Software) email: ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]

[Gavin Deane]

On 1 Jul, 10:48, James Kanze <[e-mail]:

On Jun 30, 2:24 pm, Gavin Deane <[e-mail]:

On 29 Jun, 09:57, James Kanze <[e-mail]:

On Jun 28, 8:37 pm,GavinDeane<[e-mail]:
On 28 Jun, 17:20, Angus <[e-mail]:
On 28 Jun, 16:11, Erik Wikström <[e-mail]:
The correct solution is to do like you did, use new to create the
sockets,
within an appropriate RAII object (e.g. a smart pointer),
It's hard to imagine one with the appropriate semantics for a
socket. This is exactly the sort of thing where one wants
explicit destruction.

I was only thinking about appropriate semantics for managing the
memory. If the object pointed to is responsible for a resource of its
own too (a socket connection) then it too needs to have been written
with RAII (or rather, the corollary of RAII, Destruction is Resource
Release, which is usually the real point of RAII anyway) in mind, but
that's an orthogonal point.

I don't think so. A socket has (or probably should have) a
specific lifetime; it's lifetime is not something that you can
somehow "automate", because it depends on external events.
This isn't necessarily true, however, and I can think of
scenarios, where a socket would have a lifetime which
corresponds to scope. But that isn't the case here, since the
poster is intentionally allocating them.

Raw pointers have scope and lifetime in exactly the same way as smart
pointers. The only difference is:

Use raw pointers:
While one or more pointers to the object exist, you can use the
object.
You have the option to explicitly delete the object at any time.
At the time the last pointer to the object goes out of scope, if you
haven't explicitly deleted the object, you have no other way of
deleting it so you have a memory leak.

Use smart pointers:
While one or more pointers to the object exist, you can use the
object.
You have the option to explicitly delete the object at any time.
At the time the last pointer to the object goes out of scope, if you
haven't explicitly deleted the object, the smart pointer does it for
you.

Unless you want objects to exist after you have no pointers to them
left, with no way of using or deleting them, smart pointers are better
aren't they?

I am talking here only about one of the two independent resources: the
memory in which the pointee object lives. I don't think any of the
above logic changes depending on whether the pointee object itself is
responsible for some resource of its own.

There is a second potential problem with regards to using RAII
with a socket: releasing the resource can fail, and you may have
to handle the error. In practice, this would mean that you
cannot release the resource in the destructor. Again, this
depends somewhat on design.

So the conversation goes:

Me: Hello Operating System, I would like a resource [socket
connection] please.
OS: Here you go. Remember to give it back to me won't you - that's
your responsibility.
Me: Of course I will. Thanks very much.
.... [time passes] ...
Me: Hi OS. I've finished with that resource you gave me. Here it is
back to you.
OS: No. I'm not taking it.
Me: ??? !!!

How do you handle that then? Reboot everything? Contact the people who
wrote the OS and ask if they could be more reasonable next time?

RAII, in its simplest form, works best for objects whose
lifetime depends on scope. Things like shared_ptr can be used
for memory managment, but aren't really applicable for objects
with an explicit lifetime.

I still don't get this. It's no harder to keep a smart pointer around
for as long as I want than it is to do the same for a raw pointer. And
when I need to explicitly delete the pointee, I am still left with a
dangling pointer whether I use raw or smart pointers. The only
difference is that, with smart pointers, it is not possible for the
object to still exist after I have no longer got any pointers to it.
There's never a time when you want that to be possible is there?

And of course, today, if you're
starting a new project, the only reasonable course of action is
to use the Boehm collector if you can. So shared_ptr becomes
really only pertinant to legacy applications which can't use the
Boehm collector. (I'm speaking here of the boost::shared_ptr
using the default deleter. There are some special scenarios
where it is still very, very useful with a user defined deleter
object.)

add the objects that own the
pointers to the collection, and *don't* call delete.
Then, later when you are done with the sockets
... the memory management is already fully solved and has become a non-
issue.
Memory management is best solved by using the Boehm collector.
The problem here isn't just memory management, it's lifetime
management. When does the connection exist? And it's something
that needs to be explicitly handled.

... via the interface of the class that's responsible for a socket
connection.

I was thinking at a higher level. When does the lifetime end?
As a result of what criteria.

I think there are two different things getting confused here. There
are socket connections which are opened and closed (and an open
connection is a resource I am responsible for closing). Obviously, the
lifetime of a connection is from the time it is opened to the time it
is closed (although that does become more complicated if, as you say
above, when I try to close one I might not be allowed to). And then
there are socket connection objects that manage socket connections.
The lifetime of one of these objects is whatever I choose to program.
During its lifetime, I may use such an object to open then close just
a single connection or I may use it to open then close many succesive
connections. Either way, I am responsible for ensuring that I don't
have a connection open after the object is destroyed. If it weren't
for this issue of the OS possibly not letting me give the resource
back when I want to, RAII would be the perfect tool to ensure that.

In the servers I currently work on, for example, the socket
connection's lifetime ends when the client logs out; an external
event. This is typical of a lot of TCP servers, I think. In a
client, it probably depends on what the connection is being used
for; a service connection used to obtain a specific bit of
information, then closed, will probably obey scope; a more or
less permanent connection used within a library will have an
explicit lifetime, with client code calling a specific function
to terminate its lifetime. (Certainly, the class which is
responsible for the socket will not want to listen on a specific
button to know when to close the socket.)

If the pressing of that button indicates that it's time to close the
socket connection, then *something* has to respond to the button press
by calling a close function on the socket connection manager.

Does that look right to you?
No. You're not using RAII. Someone has already suggested using a smart
pointer (boost::shared_ptr was the example).
Which for something like Socket is almost certainly a poor
choice, which will likely end up with a lot of dead objects
hanging around.

What do you mean by "dead object", how would I end up with one hanging
around, and (if it's not obvious from the definition of "dead object")
why would that be bad?

A dead object is one that still "exists", but shouldn't,
according to the program logic. It's potentially a problem with
all types of garbage collection, including boost::shared_ptr.
With normal garbage collection, however, you expect it; you
don't use the destructor to "invalidate" the object, but a
special member function, which marks the object as invalid. An
assertion check at the start of all functions then detects any
invalid use.

When it's time to close the socket connection, you call the function
that does that on the object managing that connection. Is that
connection manager now a dead object? It's design expects that some of
the time it will not have an open connection. read and write functions
will just do nothing, or perhaps set a fail flag or return a fail
code, or could have an assert in them (since attempting to read or
write while you don't have an open connection is probably a bug).
Maybe your program logic says that you have no further need for that
connection manager and so it is now a dead object. You could delete it
(assuming it was dynamically allocated) but now you just have a
dangling pointer - a different kind of dead object I presume - and so
you've gained nothing.

With shared_ptr, there is a tendancy to count on
the destructor, which results in the object not being
invalidated when it should be, because someone still has a
pointer to it hanging around.

I keep coming back to this (and so I wonder if I'm just missing the
point) but smart pointers don't remove your ability to destroy an
object while pointers to it still exist, they protect against the
possibility of not destroying an object by the time that no pointers
to it exist any more.

<snip>

Gavin Deane

[James Kanze]

Gavin Deane wrote:

On 1 Jul, 10:48, James Kanze <[e-mail]:

On Jun 30, 2:24 pm, Gavin Deane <[e-mail]:

On 29 Jun, 09:57, James Kanze <[e-mail]:
On Jun 28, 8:37 pm,GavinDeane<[e-mail]:
On 28 Jun, 17:20, Angus <[e-mail]:
On 28 Jun, 16:11, Erik Wikström <[e-mail]:
The correct solution is to do like you did, use new to createthe
sockets,
within an appropriate RAII object (e.g. a smart pointer),
It's hard to imagine one with the appropriate semantics for a
socket. This is exactly the sort of thing where one wants
explicit destruction.
I was only thinking about appropriate semantics for managing the
memory. If the object pointed to is responsible for a resource of its
own too (a socket connection) then it too needs to have been written
with RAII (or rather, the corollary of RAII, Destruction is Resource
Release, which is usually the real point of RAII anyway) in mind, but
that's an orthogonal point.

I don't think so. A socket has (or probably should have) a
specific lifetime; it's lifetime is not something that you can
somehow "automate", because it depends on external events.
This isn't necessarily true, however, and I can think of
scenarios, where a socket would have a lifetime which
corresponds to scope. But that isn't the case here, since the
poster is intentionally allocating them.

Raw pointers have scope and lifetime in exactly the same way as smart
pointers. The only difference is:

The main difference is that with raw pointers, you control the
object lifetime. With shared_ptr, and similar smart pointers,
you get whatever object lifetime the pointer decides to give
you. If object lifetime is an important part of the design
(which is probably the case with a socket class), you don't what
some smart pointer handling it. If it's not, you don't need
anything handling it.

Use raw pointers:
While one or more pointers to the object exist, you can use the
object.

What makes you say that? If the object is dead, or invalid, you
can't use it. The fact that you have a pointer to the object
doesn't guarantee it's validity in any way.

You have the option to explicitly delete the object at any time.
At the time the last pointer to the object goes out of scope, if you
haven't explicitly deleted the object, you have no other way of
deleting it so you have a memory leak.

What's all this bullshit about memory leaks. The garbage
collector (Boehm or other) takes care of memory. What we're
concerned about here is object lifetime.

Use smart pointers:
While one or more pointers to the object exist, you can use the
object.

So what you're saying is that you can output to the socket, even
if it has been closed, and---perhaps more importantly---even if
the program design says that you shouldn't. That doesn't sound
like a feature to me.

You have the option to explicitly delete the object at any time.

We must be talking about different smart pointers. Deleting an
object owned by a shared_ptr will result in a double delete.

At the time the last pointer to the object goes out of scope, if you
haven't explicitly deleted the object, the smart pointer does it for
you.

Unless you want objects to exist after you have no pointers to them
left, with no way of using or deleting them, smart pointers are better
aren't they?

No. They try to hide the problem of lifetime management, and
end up making it more difficult.

I am talking here only about one of the two independent resources: the
memory in which the pointee object lives.

In other words, you're talking about garbage collection. If for
some reason, you can't use real garbage collection, shared_ptr
can be used to simulate it. But it's a lot of extra work; you
have to worry about cycles, for example.

I don't think any of the above logic changes depending on
whether the pointee object itself is responsible for some
resource of its own.

There is a second potential problem with regards to using RAII
with a socket: releasing the resource can fail, and you may have
to handle the error. In practice, this would mean that you
cannot release the resource in the destructor. Again, this
depends somewhat on design.

So the conversation goes:

Me: Hello Operating System, I would like a resource [socket
connection] please.
OS: Here you go. Remember to give it back to me won't you - that's
your responsibility.
Me: Of course I will. Thanks very much.
... [time passes] ...
Me: Hi OS. I've finished with that resource you gave me. Here it is
back to you.
OS: No. I'm not taking it.
Me: ??? !!!

How do you handle that then?

Have you ever actually programmed any buffered IO. Closing a
socket or a file will flush the buffers. If there's an error
flushing the buffer, you have to handle it; at the very least,
you need to log the error.

RAII, in its simplest form, works best for objects whose
lifetime depends on scope. Things like shared_ptr can be used
for memory managment, but aren't really applicable for objects
with an explicit lifetime.

I still don't get this. It's no harder to keep a smart pointer around
for as long as I want than it is to do the same for a raw pointer.

As I said, things like shared_ptr can be used for memory
management. They aren't anywhere near as effective as the Boehm
collector, however.

And
when I need to explicitly delete the pointee, I am still left with a
dangling pointer whether I use raw or smart pointers. The only
difference is that, with smart pointers, it is not possible for the
object to still exist after I have no longer got any pointers to it.
There's never a time when you want that to be possible is there?

Sort of:-). If the object is registered with an external source
of events. Obviously, the event source will be keeping a
pointer to it, but in most cases, this will be as a void*, since
the event source knows nothing of your classes.

I'm not sure if this is relevant to your argument, however.
Depending on how you configure things, garbage collection may or
may not be able to track the pointer here, and you may have to
pin the object explicitly (by maintaining a pointer to it in
your own code), regardless.

If I understand you correctly, all you're arguing for is using
smart pointers for garbage collection. Better solutions exist,
and I wouldn't start a new application today without the Boehm
collector.

[...]

I think there are two different things getting confused here.
There are socket connections which are opened and closed (and
an open connection is a resource I am responsible for
closing). Obviously, the lifetime of a connection is from the
time it is opened to the time it is closed (although that does
become more complicated if, as you say above, when I try to
close one I might not be allowed to). And then there are
socket connection objects that manage socket connections. The
lifetime of one of these objects is whatever I choose to
program. During its lifetime, I may use such an object to
open then close just a single connection or I may use it to
open then close many succesive connections. Either way, I am
responsible for ensuring that I don't have a connection open
after the object is destroyed. If it weren't for this issue of
the OS possibly not letting me give the resource back when I
want to, RAII would be the perfect tool to ensure that.

The issue isn't not being able to close the socket. The issue
is handling errors that result when you close it. How do you
report an error from a destructor.

The second issue is arbitrary lifetime. If the lifetime
corresponds to scope, RAII is a perfect solution; it handles all
of the cases automatically. If the lifetime doesn't correspond
to scope, however, it doesn't really handle anything. What's
the difference between explicitly calling delete, and calling
whatever function you need to call to trigger the call to
delete.

[...]

I keep coming back to this (and so I wonder if I'm just missing the
point) but smart pointers don't remove your ability to destroy an
object while pointers to it still exist, they protect against the
possibility of not destroying an object by the time that no pointers
to it exist any more.

I'm not sure I understand what you are claiming here. Are you
saying that something like the following is legal:

class Toto
{
public:
// ...
void finished()
{
delete this ;
}
// ...
} ;

boost::shared_ptr< Toto p = new Toto ;
// ...
p->finished() ;
// ...

If so, there's something that seriously haven't understood about
boost::shared_ptr.

If, for some reason, you can't use a better garbage collector,
something like boost::shared_ptr can be used. It's just a lot
more work (and typically results in a slower program as well, at
least for the type of work I do).

--
James Kanze (GABI Software) email:ja*********@gmail.com
Conseils en informatique orientée objet/
Beratung in objektorientierter Datenverarbeitung
9 place Sémard, 78210 St.-Cyr-l'École, France, [phone]