Deleting from destructor

[mc]

Hi Experts,

This may be obvious but I can't find anything one way or another. We have
a class which is always allocated using new, e.g. Foo* foo = new Foo() so we
came up with the idea of releasing the memory from the destructor as
follows:

Foo::Foo()
{
// Initialize stuff
m_This = this; // m_This is a "void*"
}

Foo::~Foo()
{
// Release all resources
// and finally the memory
delete m_This;
}

It's been working fine so far (both Windows and Linux) but we're wondering
about it being either the worse thing to do..... Any thoughts.

Thank you.

Regards,

MC

 

[mc]

Thanks Victor. I understand what you said and knew. Let me put more
context here but adding a more complete example:

void SKEL::bind(const MCU& mcu, ...)
{
// const FOO& MCU::foo()
// {
// return (*new Foo());
// }
FOO foo = mcu.foo(); // Because of the const FOO&
returned, foo becomes equal to what was returned by MCU::foo()

// do stuff
// when exising here, the destructor for FOO is called and the memory is
release as per previous post
}

The destructor for the object is only called once and no memory leaks were
detected.

MC

 

[Rolf Magnus]

Thanks Victor. I understand what you said and knew. Let me put more
context here but adding a more complete example:

void SKEL::bind(const MCU& mcu, ...)
{
// const FOO& MCU::foo()
// {
// return (*new Foo());
// }
FOO foo = mcu.foo(); // Because of the const FOO&
returned, foo becomes equal to what was returned by MCU::foo()

Yes. It becomes a copy of it.

// do stuff
// when exising here, the destructor for FOO is called and the memory
is release as per previous post

The memory for the object foo in SKEL::bind is, but there is the other FOO
object that was dynamically allocated. It's still hanging around, and you
lost all pointers to it, so you can't ever deallocate it. That's a memory
leak. With the destructor you described, you also happen to use delete with
a pointer to memory you didn't get from new, which results in undefined
behavior.

}

The destructor for the object is only called once and no memory leaks were
detected.

Sounds like your memory debugger has a problem.

 

[mc]

I haven't lost the location where the object was; remember that in the
constructor a private member is initialized to the value of where the object
resides in memory; and the destructor uses a delete for that.

 

[Rolf Magnus]

Please don't top-post.

I haven't lost the location where the object was; remember that in the
constructor a private member is initialized to the value of where the
object resides in memory; and the destructor uses a delete for that.

Yes, you're right. I missed that. So the copy's destructor frees the
original object's memory. However, the original's destructor isn't properly
called as it's supposed to, which probably won't lead to problems in the
example you showed, but will in less trivial programs. This is very bad
style.

 

[mc]

Why is there a need for the original's destructor to be called? After all,
the original is copied verbatim and consequently the destructor is called
once as expected.

Be that as it may, this is indeed quite obfuscated. The reasoning for
that is that the main caller does not know the object is being allocated
using new and hence is not supposed to call delete. So we had to find a way
to free the memory used and went for a new in place (see actual code below):

const Region& MCU::buffer(void) const
{
char* p = new char[sizeof(Region)];
return (*new(p) Region(std::string(TEMP), 1500));
}



with the Region ctdt:

Region::Region(const std::string& name, size_t size)
{
_this = reinterpret_cast<char *>(this);
_cache = new unsigned char[size], _offset = 0;
_fd = :pen(name.c_str(), O_RDWR | O_CREAT | O_TRUNC, S_IRUSR | S_IWUSR
| S_IRGRP | S_IROTH);
assert(_fd != -1);
}


Output::Buffer::~Buffer()
{
if (_fd != -1)
{
::write(_fd, (char *)_cache, _offset);
::close(_fd);
}
delete[] _cache;
delete[] _this;
}



Does that make sense?

 

[Kai-Uwe Bux]

You did it again. Please do not top-post. It is frowned upon around these
parts.

Why is there a need for the original's destructor to be called? After all,
the original is copied verbatim and consequently the destructor is called
once as expected.

[snip]

I am not sure, I understand your code. You have:

Foo::Foo() {
// Initialize stuff
m_This = this; // m_This is a "void*"
}

Foo::~Foo() {
// Release all resources
// and finally the memory
delete m_This;
}

const FOO& MCU::foo() {
return (*new Foo());
}

and then

FOO foo = mcu.foo(); // creates a copy

Now the destructor of the foo object will be called. This destructor, in
turn, is going to invoke

delete m_This;

which will call the destructor for the original. Here is what I do not
understand:

a) Why is m_This a void*? It appears that in this case

delete m_This;

will _not_ invoke the destructor of the original.

b) Why are you not running into an infinite loop? After all, the destructor
of the original, once invoked, has to execute

delete m_This;

too.

c) Could it be that (a) and (b) are related, i.e., do you run into an
infinite loop if m_This had type foo*? If that is the case, then with the

delete m_This;

statement in the destructor of the copy foo, you are likely invoking
undefined behavior because of a type mismatch of the actual object and what
you are telling the compiler [5.3.5/3].


Best

Kai-Uwe Bux

 

[mc]

Sorry for being opaque but I have no idea what top-posting is....

 

[Ian Collins]

Sorry for being opaque but I have no idea what top-posting is....

Ever heard of Google?

 

[Default User]

Sorry for being opaque but I have no idea what top-posting is....

<http://en.wikipedia.org/wiki/Top_posting>

See also:

<http://www.parashift.com/c++-faq-lite/how-to-post.html>




Brian

 

[mc]

<http://en.wikipedia.org/wiki/Top_posting>

See also:

<http://www.parashift.com/c++-faq-lite/how-to-post.html>




Brian

Thank you.

 

[Rolf Magnus]

Why is there a need for the original's destructor to be called?

For the otherwise empty object you have here, it might be ok (I don't know
if it's sanctioned by the standard), but when your object gets more members
that are themselves class instances, you will run into trouble.

After all, the original is copied verbatim and consequently the destructor
is called once as expected.

But you have two objects. One constructed by the default constructor, the
other constructed by the copy constructor. And each of those two objects is
supposed to be constructed once and destroyed once.

Be that as it may, this is indeed quite obfuscated. The reasoning for
that is that the main caller does not know the object is being allocated
using new and hence is not supposed to call delete.

Why do you have the dynamically allocated object at all? Just return by
value, and you get the same, but in a clean way.

So we had to find a way to free the memory used and went for a new in
place (see actual code below):

const Region& MCU::buffer(void) const
{
char* p = new char[sizeof(Region)];
return (*new(p) Region(std::string(TEMP), 1500));
}

Why not simply:

const Region MCU::buffer(void) const
{
return Region(std::string(TEMP), 1500);
}

 

[Aggro]

Hi Experts,

This may be obvious but I can't find anything one way or another. We have
a class which is always allocated using new, e.g. Foo* foo = new Foo() so we
came up with the idea of releasing the memory from the destructor as
follows:

Foo::Foo()
{
// Initialize stuff
m_This = this; // m_This is a "void*"
}

Foo::~Foo()
{
// Release all resources
// and finally the memory
delete m_This;
}

It's been working fine so far (both Windows and Linux) but we're wondering
about it being either the worse thing to do..... Any thoughts.

Put std::cout << "hello start" << std::endl; to your constructor and
similar "hello end" to your destructor. What do you see when you execute
the program? Do you see both start and end?

If you do, please post complite example (fitting inside a single file)
that can be compiled, because I fail to see how the destructor would be
called at all so I probably don't understand your explanation.

 

[Noah Roberts]

mc wrote:

It's an absolutely insane thing to do. The destructor is not called
unless you call delete or you leave scope on a stack variable. In
either of those cases you do not need to delete your object again.
Furthermore, deleting from a void* doesn't work.

I very much doubt it's been working as "fine" as you think.