View mode: basic / threaded / horizontal-split · Log in · Help
March 25, 2010
Never called destructor
Why, in the following piece of code, destructor for 'x' is never called?

   class A{ char [] s; this(char [] o_s){ s = o_s; } ~this(){ writefln(s); }; }

void main(){
   A y;
   {
   scope A x = new A("x".dup); y = new A("y".dup);
   x = y; // 
   }
}

output:
y // where is x

Destructors for scope objects are executed, because:

void main(){
   A y;
   {
   scope A x = new A("x".dup); y = new A("y".dup);
   // x = y; 
   }
}
output:
x
y

and for assignment too:

void main(){
   A y;
   {
   A x = new A("x".dup); y = new A("y".dup);
   x = y; 
   }
}
output:
y 
x
March 26, 2010
Re: Never called destructor
Zarathustra wrote:

> Why, in the following piece of code, destructor for 'x' is never called?
> <snip>

Because it is not guaranteed to be called.

http://www.digitalmars.com/d/2.0/class.html#destructors

According to the spec, for the cases you've provided,
you are only guaranteed to see:

Y // for the first case
X // for the second case
<nothing> // at all for the third case.

===

So: (rejigged to remove some pointless statements)

class A{
 string s;
 this(string o_s){
   s = o_s;
 }
 ~this(){
   writefln(s);
 }
}

Case 1:

void main(){
 A y;
 {
   scope x = new A("x");
   y = new A("y");
   x = y;
 } // end scope
}

"scope x", makes the compiler delete the instance held in x when the
end of scope is reached, so you see Y as that is the instance held in x.
The A with "x" is lost to the garbage collector as you reassigned the
reference 'x'.

You don't see x in the output, because the destructor for the instance
with "x" is not guaranteed to be run and in this case it isn't.

Case 2:

void main(){
 A y;
 {
   scope x = new A("x".dup);
   y = new A("y".dup);
 } // end scope
}

see above, "x" is in x this time so you see that deleted.

For *some reason* which is not explained and *can not* be relied upon,
according to the spec, you get a delete of y.

This maybe because the compiler knows through static analysis that both
instances can be deleted.

Case 3:

pretty much the same as 2, but we don't have scope so we aren't
guaranteed to see either destructor. it would be perfectly valid
according to the spec to not see any destructor called at all.

That fact that you see them both called is not easily explainable unless
you have a deep understanding of the compiler and/or the druntime.

I personally would go a bit further and say that you shouldn't see
either destructor called in the third case and shouldn't see a
destructor called for y in the second one.

They aren't guaranteed to be called; so you can't depend on any side
affects; so the druntime should not bother to run a garbage collect on
exit. IIRC in C# apps, you don't see destructors run when you exit.

Deleting & freeing memory when the program is exiting is a complete
waste of time; the OS will reclaim all the memory once your app has died.

- --
My enormous talent is exceeded only by my outrageous laziness.
http://www.ssTk.co.uk
March 26, 2010
Re: Never called destructor
div0:
>     scope x = new A("x");
>     y = new A("y");
>     x = y;

In my opinion it's better to not reassign references of scoped objects.
In real programs where possible it's better to write boring and stupid code :-)

Bye,
bearophile
March 26, 2010
Re: Never called destructor
bearophile wrote:
> div0:
>>     scope x = new A("x");
>>     y = new A("y");
>>     x = y;
> 
> In my opinion it's better to not reassign references of scoped objects.
> In real programs where possible it's better to write boring and stupid code :-)
> 
> Bye,
> bearophile

Yeah, I was thinking about that and wondering whether in fact it should
be an error and disallowed.

I use scope because I want the instance on the stack for performance,
and allowing the scope ref to be reassigned buggers things up.

also consider:

import std.stdio;

class A {
	string	_instance;

	this(string instance) {
		_instance = instance;
	}

	~this() {
		writefln("A.~this @ 0x%x: %s", cast(void*)this, _instance);
	}
}

A test() {
	scope	x = new A("x");
	auto	y = new A("y");
	x = y;
	return y;
}

void main()
{
	scope z = test();
	writefln("main, z @ 0x%x, [%s]", cast(void*)z, z._instance);
}

output:

A.~this @ 0x962E40: y
main, z @ 0x962E40, [y]

This is clearly wrong, we are accessing a deleted object, and for some
reason we aren't getting a double delete of y, which we should.

- --
My enormous talent is exceeded only by my outrageous laziness.
http://www.ssTk.co.uk
March 26, 2010
Re: Never called destructor
div0:
> This is clearly wrong, we are accessing a deleted object, and for some
> reason we aren't getting a double delete of y, which we should.

Thank you for the nice example, I think it's doing the struct return optimization trick invented by Walter ages ago :-)

"scope" for objects is a very useful optimization, I can show you benchmarks, because DMD isn't able to perform escape analysis as recent HotSpot does. But I think Walter knows it's not safe (it can be made quite more safe if more sanity tests are added to DMD, but this decreases their flexibility and usefulness when you need more performance).

So don't use it to increase code safety, because it fails at that. Use it only where you can experimentally see a performance improvement. And where possible don't copy, reassign or return the reference of a scoped object :-) Objects in D are not meant to be copied. And the purpose of a scoped object is to exist only in a scope, to avoid a heap allocation. If you try to break free of this semantics you are on your own.

Do you want to write a bug report? I fear that the only way to make scoped objects fully semantically sound is to remove them from the language, and Walter was about to do it. But there are legit usages for scoped objects.

Bye,
bearophile
March 26, 2010
Re: Never called destructor
div0 wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> 
> bearophile wrote:
>> div0:
>>>     scope x = new A("x");
>>>     y = new A("y");
>>>     x = y;
>> In my opinion it's better to not reassign references of scoped objects.
>> In real programs where possible it's better to write boring and stupid code :-)
>>
>> Bye,
>> bearophile
> 
> Yeah, I was thinking about that and wondering whether in fact it should
> be an error and disallowed.
> 
> I use scope because I want the instance on the stack for performance,
> and allowing the scope ref to be reassigned buggers things up.
> 
> also consider:
> 
> import std.stdio;
> 
> class A {
> 	string	_instance;
> 
> 	this(string instance) {
> 		_instance = instance;
> 	}
> 
> 	~this() {
> 		writefln("A.~this @ 0x%x: %s", cast(void*)this, _instance);
> 	}
> }
> 
> A test() {
> 	scope	x = new A("x");
> 	auto	y = new A("y");
> 	x = y;
> 	return y;
> }
> 
> void main()
> {
> 	scope z = test();
> 	writefln("main, z @ 0x%x, [%s]", cast(void*)z, z._instance);
> }
> 
> output:
> 
> A.~this @ 0x962E40: y
> main, z @ 0x962E40, [y]
> 
> This is clearly wrong, we are accessing a deleted object, and for some
> reason we aren't getting a double delete of y, which we should.

Same as bug 3285 / bug 3516?
March 27, 2010
Re: Never called destructor
Don wrote:
> div0 wrote:
>> -----BEGIN PGP SIGNED MESSAGE-----
>> Hash: SHA1
> 
> Same as bug 3285 / bug 3516?

No, they are for structs, not classes.

I had a bit more of a play, and it seems that the scope object is on the
stack, so it's memory is reclaimed, but the destructor is called for
the y object instead.

It would be much simpler just to disallow reassignment of the scope var
and probably it should not be allowed to assign the ref of a scope
object to another reference either to stop you accidentally escaping the
ref.

Also 3566 is a related bug. So delete of a scope var needs to go as well.

- --
My enormous talent is exceeded only by my outrageous laziness.
http://www.ssTk.co.uk
Top | Discussion index | About this forum | D home