www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Bug in RefCounted?

reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
I'm writing a D wrapper for a C library. I was planning to use 
RefCounted structs to control the lifetime of objects created by 
this library. Please check the following example:

http://dpaste.dzfl.pl/b49962bf

Foo would be an opaque struct. createFoo() and destroyFoo() would 
be implemented in the C library (I know I have to declare them 
extern(C), this is just an example).

As you can see, that code prints 'Destroying Foo' twice, with 
different pointers. I expected destroyFoo to be called only once, 
on the instance created by createFoo(), when the Bar instance 
goes out of scope. Current behaviour causes an invalid pointer to 
be passed to destroyFoo(). Is this a bug in RefCounted or am I 
doing something wrong?
Oct 24 2013
next sibling parent reply "Jesse Phillips" <Jesse.K.Phillips+D gmail.com> writes:
On Thursday, 24 October 2013 at 14:58:21 UTC, Rene Zwanenburg 
wrote:

 I'm writing a D wrapper for a C library. I was planning to use 
 RefCounted structs to control the lifetime of objects created 
 by this library. Please check the following example:

 http://dpaste.dzfl.pl/b49962bf

 Foo would be an opaque struct. createFoo() and destroyFoo() 
 would be implemented in the C library (I know I have to declare 
 them extern(C), this is just an example).

 As you can see, that code prints 'Destroying Foo' twice, with 
 different pointers. I expected destroyFoo to be called only 
 once, on the instance created by createFoo(), when the Bar 
 instance goes out of scope. Current behaviour causes an invalid 
 pointer to be passed to destroyFoo(). Is this a bug in 
 RefCounted or am I doing something wrong?


I answered a question related to RefCount on SO


Not written to your specific problem, but may give you the 
information you need.
Oct 24 2013
parent "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
On Thursday, 24 October 2013 at 16:40:42 UTC, Jesse Phillips 
wrote:

 I answered a question related to RefCount on SO


 Not written to your specific problem, but may give you the 
 information you need.


Thanks, that did help. Still, I think there's something strange 
going on.

The problem is that RefCounted calls the FooWrapper destructor 
twice, one time with a garbage foo pointer. The example prints:
Destroying Foo 40CED748
Destroying Foo 4002DFF0
There is only one Foo instance created. A null pointer would make 
sense, but where's that second pointer coming from?
Oct 27 2013
prev sibling parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 10/24/2013 07:58 AM, Rene Zwanenburg wrote:

 I'm writing a D wrapper for a C library. I was planning to use
 RefCounted structs to control the lifetime of objects created by this
 library. Please check the following example:

 http://dpaste.dzfl.pl/b49962bf

 Foo would be an opaque struct. createFoo() and destroyFoo() would be
 implemented in the C library (I know I have to declare them extern(C),
 this is just an example).

 As you can see, that code prints 'Destroying Foo' twice, with different
 pointers. I expected destroyFoo to be called only once, on the instance
 created by createFoo(), when the Bar instance goes out of scope. Current
 behaviour causes an invalid pointer to be passed to destroyFoo(). Is
 this a bug in RefCounted or am I doing something wrong?


Technically, it is a problem with FooWrapper. Regardless of whether 
RefCounted's behavior, by default, structs in D are freely copyable and 
movable value types. The compiler can do those things as it sees fit.

Since FooWrapper owns a resource, it must also define the post-blit to 
make a copy of Foo.

(As an aside, dmd at git head does not make such a copy.)

Ali
Oct 24 2013
parent reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
On Thursday, 24 October 2013 at 16:46:37 UTC, Ali Çehreli wrote:

 Technically, it is a problem with FooWrapper. Regardless of 
 whether RefCounted's behavior, by default, structs in D are 
 freely copyable and movable value types. The compiler can do 
 those things as it sees fit.

 Since FooWrapper owns a resource, it must also define the 
 post-blit to make a copy of Foo.

 (As an aside, dmd at git head does not make such a copy.)

 Ali


Thanks, that's indeed an oversight on my part. Still, I think 
there's something strange going on.

The problem is that the FooWrapper destructor is called twice, 
one time with a garbage foo pointer. The example prints:
Destroying Foo 40CED748
Destroying Foo 4002DFF0
There is only one Foo instance created. A null pointer would make 
sense, but where's that second pointer coming from?
Oct 27 2013
parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Sunday, 27 October 2013 at 21:02:01 UTC, Rene Zwanenburg wrote:

 On Thursday, 24 October 2013 at 16:46:37 UTC, Ali Çehreli wrote:

 Technically, it is a problem with FooWrapper. Regardless of 
 whether RefCounted's behavior, by default, structs in D are 
 freely copyable and movable value types. The compiler can do 
 those things as it sees fit.

 Since FooWrapper owns a resource, it must also define the 
 post-blit to make a copy of Foo.

 (As an aside, dmd at git head does not make such a copy.)

 Ali


 Thanks, that's indeed an oversight on my part. Still, I think 
 there's something strange going on.

 The problem is that the FooWrapper destructor is called twice, 
 one time with a garbage foo pointer. The example prints:
 Destroying Foo 40CED748
 Destroying Foo 4002DFF0
 There is only one Foo instance created. A null pointer would 
 make sense, but where's that second pointer coming from?


This is indeed strange. I don't have access to my development
platform, but it *could* be an (older) emplace bug. Do you
reproduce with head? I'll investigate further as soon as I can.
This is not normal behavior.

Also, keep in mind that DMD *is* allowed to destroy the same
object several times. YOur destructor should look like this:
~this()
{
      if (foo)
          destroyFoo(foo);
      foo = null;
}

This doesn't explain what you are seeing, but keep it in mind.
Oct 27 2013
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 10/27/2013 03:04 PM, monarch_dodra wrote:


 it *could* be an (older) emplace bug. Do you reproduce with head?


I had tested it with head. No, doesn't happen on head.


 Also, keep in mind that DMD *is* allowed to destroy the same
 object several times.


That's news to me. I know that objects may never be destroyed but why 
multiple times? How many lives do they have? ;)


 YOur destructor should look like this:



 ~this()
 {
       if (foo)
           destroyFoo(foo);
       foo = null;
 }


Welcome back to C. :p

Ali
Oct 27 2013
parent reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
On Sunday, 27 October 2013 at 23:33:55 UTC, Ali Çehreli wrote:

 On 10/27/2013 03:04 PM, monarch_dodra wrote:


 it *could* be an (older) emplace bug. Do you reproduce with


 head?

 I had tested it with head. No, doesn't happen on head.


Thanks for testing that. I'm not set up to build dmd or phobos 
myself.


 Also, keep in mind that DMD *is* allowed to destroy the same
 object several times.


 That's news to me. I know that objects may never be destroyed 
 but why multiple times? How many lives do they have? ;)


Yeah, I'd like to know this as well. I do remember structs 
allocated on the heap don't have their destructors called at all 
due to the GC not being precise, I think.

But one object allowed to be destructed multiple times? That 
sounds really bad.. If that's true a lot of my code is probably 
incorrect.
Oct 28 2013
next sibling parent reply "monarch_dodra" <monarchdodra gmail.com> writes:
On Monday, 28 October 2013 at 10:07:15 UTC, Rene Zwanenburg wrote:

 Yeah, I'd like to know this as well. I do remember structs 
 allocated on the heap don't have their destructors called at 
 all due to the GC not being precise, I think.

 But one object allowed to be destructed multiple times? That 
 sounds really bad.. If that's true a lot of my code is probably 
 incorrect.


Hum... I seem to remember having replied earlier, but I guess I
forgot to hit send.

In any case, I could be mistaken, but I simply know that under
certain circumstances, it can happen. I don't know if that's a
bug though. I'll try to find the cases where it happens.

Furthermore, you must *always* make sure that the T.init state is
destroyable (which is not the case here).
Oct 28 2013
parent reply "Maxim Fomin" <maxim maxim-fomin.ru> writes:
On Monday, 28 October 2013 at 16:53:11 UTC, monarch_dodra wrote:

 On Monday, 28 October 2013 at 10:07:15 UTC, Rene Zwanenburg 
 wrote:

 Yeah, I'd like to know this as well. I do remember structs 
 allocated on the heap don't have their destructors called at 
 all due to the GC not being precise, I think.

 But one object allowed to be destructed multiple times? That 
 sounds really bad.. If that's true a lot of my code is 
 probably incorrect.


 Hum... I seem to remember having replied earlier, but I guess I
 forgot to hit send.

 In any case, I could be mistaken, but I simply know that under
 certain circumstances, it can happen.


OK


 I don't know if that's a
 bug though. I'll try to find the cases where it happens.


So do you *know* cases or suspect that they may exists? Or 
remember some bug issue?

Here is my attempt:

import std.stdio;

struct S
{
    int i;
    this(int i)   { writefln("ctor, %X", i); this.i = i; }
    this(this)  { writefln("postblit, %X, %X", &this, i); }
    ~this()     { writefln("dtor, %X, %X", &this, i); }
}

auto foo()
{
    S s = S(1);
    return { s = S(2); } ;
}

void main()
{
    foo()();
}

ctor, 1
dtor, 7FFFF7ED8FF8, 1
ctor, 2
dtor, 7FFFFFFFDB30, 1

Inside foo() function object 's' is destroyed twice: first time 
as a regular struct at the end of block scope, second time before 
assigning S(2).

There are other tools: union bug, control flow tricks, __traits, 
__dtor but they are move obvious.
Oct 28 2013
next sibling parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 10/28/2013 12:30 PM, Maxim Fomin wrote:


 So do you *know* cases or suspect that they may exists? Or remember some
 bug issue?

 Here is my attempt:

 import std.stdio;

 struct S
 {
     int i;
     this(int i)   { writefln("ctor, %X", i); this.i = i; }
     this(this)  { writefln("postblit, %X, %X", &this, i); }
     ~this()     { writefln("dtor, %X, %X", &this, i); }
 }

 auto foo()
 {
     S s = S(1);
     return { s = S(2); } ;
 }

 void main()
 {
     foo()();
 }

 ctor, 1
 dtor, 7FFFF7ED8FF8, 1
 ctor, 2
 dtor, 7FFFFFFFDB30, 1

 Inside foo() function object 's' is destroyed twice: first time as a
 regular struct at the end of block scope, second time before assigning
 S(2).


Ok, that's too much! :( Inspired by your program, the following delegate 
completely misses the point:

import std.stdio;

struct S
{
     int i;
     ~this() { i = 666; }
}

auto foo()
{
     S s = S(1);

     // When I see the following delegate, I think "the lifetime of s is
     // extended." I expect my delegate to print 1.
     return { writeln(s); } ;
}

void main()
{
     // Unfortunately, the following prints 666!
     foo()();
}

What is the purpose of writeln in that delegate? Obviously, to print 1. 
Yet it doesn't happen that way. Is this accepted to be a bug? Should the 
programmer 'new' the object instead?

Ali
Oct 28 2013
parent reply "Maxim Fomin" <maxim maxim-fomin.ru> writes:
On Monday, 28 October 2013 at 20:43:01 UTC, Ali Çehreli wrote:

 What is the purpose of writeln in that delegate? Obviously, to 
 print 1. Yet it doesn't happen that way. Is this accepted to be 
 a bug? Should the programmer 'new' the object instead?

 Ali


In my opinion it is a corner case, a consequence of two things: 
1) need to allocate struct into heap due to lambda 2) need to put 
dtor invocation in the end as usual.
Oct 29 2013
parent reply =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 10/29/2013 08:47 AM, Maxim Fomin wrote:

 On Monday, 28 October 2013 at 20:43:01 UTC, Ali Çehreli wrote:

 What is the purpose of writeln in that delegate? Obviously, to print
 1. Yet it doesn't happen that way. Is this accepted to be a bug?
 Should the programmer 'new' the object instead?

 Ali


 In my opinion it is a corner case, a consequence of two things: 1) need
 to allocate struct into heap due to lambda 2) need to put dtor
 invocation in the end as usual.


As a continuation of this sub thread, I've opened the following thread 
in the D newsgroup:

   http://forum.dlang.org/post/l4osr0$2f3q$1 digitalmars.com

Ali
Oct 29 2013
parent reply "Maxim Fomin" <maxim maxim-fomin.ru> writes:
On Tuesday, 29 October 2013 at 17:57:01 UTC, Ali Çehreli wrote:

 On 10/29/2013 08:47 AM, Maxim Fomin wrote:

 On Monday, 28 October 2013 at 20:43:01 UTC, Ali Çehreli wrote:

 What is the purpose of writeln in that delegate? Obviously, 
 to print
 1. Yet it doesn't happen that way. Is this accepted to be a 
 bug?
 Should the programmer 'new' the object instead?

 Ali


 In my opinion it is a corner case, a consequence of two 
 things: 1) need
 to allocate struct into heap due to lambda 2) need to put dtor
 invocation in the end as usual.


 As a continuation of this sub thread, I've opened the following 
 thread in the D newsgroup:

   http://forum.dlang.org/post/l4osr0$2f3q$1 digitalmars.com

 Ali


I did this in 2 January 
(http://forum.dlang.org/thread/lpljpfjxwobniglwnqvl forum.dlang.org) 
and received exactly zero responces.
Oct 29 2013
parent =?UTF-8?B?QWxpIMOHZWhyZWxp?= <acehreli yahoo.com> writes:
On 10/29/2013 11:11 AM, Maxim Fomin wrote:


 On Tuesday, 29 October 2013 at 17:57:01 UTC, Ali Çehreli wrote:



   http://forum.dlang.org/post/l4osr0$2f3q$1 digitalmars.com





 I did this in 2 January
 (http://forum.dlang.org/thread/lpljpfjxwobniglwnqvl forum.dlang.org) and
 received exactly zero responces.


I will respond to yours after my redundant one settles. :p

Ali
Oct 29 2013
prev sibling parent reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
On Monday, 28 October 2013 at 19:30:12 UTC, Maxim Fomin wrote:

 Here is my attempt:

 import std.stdio;

 struct S
 {
    int i;
    this(int i)   { writefln("ctor, %X", i); this.i = i; }
    this(this)  { writefln("postblit, %X, %X", &this, i); }
    ~this()     { writefln("dtor, %X, %X", &this, i); }
 }

 auto foo()
 {
    S s = S(1);
    return { s = S(2); } ;
 }

 void main()
 {
    foo()();
 }

 ctor, 1
 dtor, 7FFFF7ED8FF8, 1
 ctor, 2
 dtor, 7FFFFFFFDB30, 1

 Inside foo() function object 's' is destroyed twice: first time 
 as a regular struct at the end of block scope, second time 
 before assigning S(2).

 There are other tools: union bug, control flow tricks, 
 __traits, __dtor but they are move obvious.


That's pretty nasty :). But I suspect this is a bug and not by 
design. __dtor and __traits are, IMHO, the proverbial escape 
hatch D should provide, so I think that's OK. I take it that by 
control flow trick you mean the try/catch example in your other 
post?

Anyway, thanks for pointing this out. Will probably save me some 
debugging in the future.
Oct 29 2013
next sibling parent "Maxim Fomin" <maxim maxim-fomin.ru> writes:
On Tuesday, 29 October 2013 at 11:46:53 UTC, Rene Zwanenburg 
wrote:

 That's pretty nasty :). But I suspect this is a bug and not by 
 design. __dtor and __traits are, IMHO, the proverbial escape 
 hatch D should provide, so I think that's OK. I take it that by 
 control flow trick you mean the try/catch example in your other 
 post?


By control flow tricks I mean follows: compiler inserts dtor 
invocation in the end the function for stack structs and static 
arrays of struct, so in theory one way to call dtor twice is to 
jump multiple times just before dtor call, using for example 
gotos, exceptions or such obscure feature as C setjmp/longjump.
Oct 29 2013
prev sibling parent "Kenji Hara" <k.hara.pg gmail.com> writes:
On Tuesday, 29 October 2013 at 11:46:53 UTC, Rene Zwanenburg 
wrote:

 On Monday, 28 October 2013 at 19:30:12 UTC, Maxim Fomin wrote:

 Here is my attempt:

 import std.stdio;

 struct S
 {
   int i;
   this(int i)   { writefln("ctor, %X", i); this.i = i; }
   this(this)  { writefln("postblit, %X, %X", &this, i); }
   ~this()     { writefln("dtor, %X, %X", &this, i); }
 }

 auto foo()
 {
   S s = S(1);
   return { s = S(2); } ;
 }

 void main()
 {
   foo()();
 }

 ctor, 1
 dtor, 7FFFF7ED8FF8, 1
 ctor, 2
 dtor, 7FFFFFFFDB30, 1

 Inside foo() function object 's' is destroyed twice: first 
 time as a regular struct at the end of block scope, second 
 time before assigning S(2).

 There are other tools: union bug, control flow tricks, 
 __traits, __dtor but they are move obvious.


 That's pretty nasty :). But I suspect this is a bug and not by 
 design. __dtor and __traits are, IMHO, the proverbial escape 
 hatch D should provide, so I think that's OK. I take it that by 
 control flow trick you mean the try/catch example in your other 
 post?

 Anyway, thanks for pointing this out. Will probably save me 
 some debugging in the future.


The combination of closure variables + scoped destruction should 
be rejected, but currently it isn't. It's a compiler bug.

http://d.puremagic.com/issues/show_bug.cgi?id=11382

Kenji Hara
Oct 29 2013
prev sibling parent reply "Maxim Fomin" <maxim maxim-fomin.ru> writes:
On Monday, 28 October 2013 at 10:07:15 UTC, Rene Zwanenburg wrote:

 On Sunday, 27 October 2013 at 23:33:55 UTC, Ali Çehreli wrote:

 That's news to me. I know that objects may never be destroyed 
 but why multiple times? How many lives do they have? ;)


 Yeah, I'd like to know this as well. I do remember structs 
 allocated on the heap don't have their destructors called at 
 all due to the GC not being precise, I think.


The fact that structs are movable and there is too few struct 
runtime reflection makes them noncollectable. However, you can 
wrap struct inside class, in such case struct dtor will be called.


 But one object allowed to be destructed multiple times? That 
 sounds really bad.. If that's true a lot of my code is probably 
 incorrect.


I think one need to be aware of this bug (no dtor), rather than 
cases when dtor is called multiple times.

import std.stdio;

struct S
{
    int i;
    this(int i)   { writefln("ctor, %X", i); this.i = i; }
    this(this)  { writefln("postblit, %X, %X", &this, i); }
    ~this()     { writefln("dtor, %X, %X", &this, i); }
}

int this_throws() { throw new Exception(""); }

void foo(S s, int i) {}

void main()
{
    try { foo(S(1), this_throws); }
    catch(Exception e) {}
}
Oct 28 2013
parent reply "Rene Zwanenburg" <renezwanenburg gmail.com> writes:
On Monday, 28 October 2013 at 19:40:26 UTC, Maxim Fomin wrote:

 The fact that structs are movable and there is too few struct 
 runtime reflection makes them noncollectable. However, you can 
 wrap struct inside class, in such case struct dtor will be 
 called.


Yeah, if wrapping inside a class wouldn't work either we'd be in 
a whole new world of hurt.

But what do you exactly mean by noncollectable? And what does 
movability have to do with that? I think the memory will be 
reclaimed without a problem, so new-ing a struct without 
destructor would be fine. This doesn't leak:

struct S
{
	int i;
}

void main()
{
	while(true)
	{
		new S;
	}
}
Oct 29 2013
parent "Maxim Fomin" <maxim maxim-fomin.ru> writes:
On Tuesday, 29 October 2013 at 12:03:09 UTC, Rene Zwanenburg 
wrote:

 On Monday, 28 October 2013 at 19:40:26 UTC, Maxim Fomin wrote:

 The fact that structs are movable and there is too few struct 
 runtime reflection makes them noncollectable. However, you can 
 wrap struct inside class, in such case struct dtor will be 
 called.


 Yeah, if wrapping inside a class wouldn't work either we'd be 
 in a whole new world of hurt.

 But what do you exactly mean by noncollectable? And what does 
 movability have to do with that? I think the memory will be 
 reclaimed without a problem, so new-ing a struct without 
 destructor would be fine. This doesn't leak:

 struct S
 {
 	int i;
 }

 void main()
 {
 	while(true)
 	{
 		new S;
 	}
 }


I would say it leaks because dtor (if defined) is not called, 
although memory is reclaimed at some point. In my opinion struct 
movability is a problem for heap struct collection because it is 
hard to say judging to struct stack/heap pointer whether it 
should be collected or not. If you look at output from previous 
example,

ctor, 1
dtor, 7FFFF7ED8FF8, 1
ctor, 2
dtor, 7FFFFFFFDB30, 1

you will see that adressess are different and refer to stack, yet 
object is in the heap. By the way, there is another issue - if 
you have delegate inside struct method touching some field, than 
invoking such delegate after returning from method may break 
memory, because delegate references 'this' struct pointer, but it 
may be changed across lifetime. I am not sure whether some 
algorithm for solving heap struct collectibility can be made, but 
I don't have any idea right now.
Oct 29 2013