www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.bugs - [Issue 6581] New: Yet another dtor/postblit problem?

reply d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581

           Summary: Yet another dtor/postblit problem?
           Product: D
           Version: D2
          Platform: Other
        OS/Version: All
            Status: NEW
          Severity: major
          Priority: P2
         Component: DMD
        AssignedTo: nobody puremagic.com
        ReportedBy: dmitry.olsh gmail.com


--- Comment #0 from Dmitry Olshansky <dmitry.olsh gmail.com> 2011-08-30
14:12:40 PDT ---
Test case creates a instance of nested struct, and prints ctor/postblit/dtor
activity + address of this. Reduced from upcoming regex module, where it's
causing segfaults but only with -inline (dumb luck?).

import std.stdio;
struct R
{
    this(int k)
    {
        writefln("R created %x", &this);
    }
    this(this){ writefln("R postblit %x", &this); }

    ~this(){  writefln("R destroyed %x", &this); }
}

struct S
{
    R _match;

    this( int separator)
    {
      _match = R(separator);
      writefln("S created %x", cast(void*)&this);
    }
    this(this)
    {
      writefln("S postblit %x", cast(void*)&this);
    }
    ~this()
    {
      writefln("S destroyed %x", cast(void*)&this);
    }
}

void split(int rx)
{
    auto spl = S(rx);
    writefln("**** Spl is at address %x", cast(void*)&spl);
}

void main(string[] argv)
{
    split(42);
}


The end result for me looks like this:

R created 18fdc0
R destroyed 18fdc4  // killed wrong guy or missing a postblit at 18fe08?
S created 18fe08
**** Spl is at address 18fe08
S destroyed 18fe08
R destroyed 18fe08 

Bottom line: dtor called twice, ctor called once and not a single postblit. 
Saldo is negative ...

That's on almost latest DMD (commit c5c8500a13f222935f00145c16dfbc2d32351b0f)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Aug 30 2011
next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #1 from Dmitry Olshansky <dmitry.olsh gmail.com> 2011-09-01
13:21:38 PDT ---
Simplifyied. My best guess is that postlblits for struct members are not
called.

struct A
{
    static int cnt;
    this(int dummy){ cnt++; }
    this(this){ cnt++; }
    ~this(){ cnt--; }
}
struct B
{
    A a;
    static int cnt;
    this(int dummy){ a = a(dummy); cnt++; }
    this(this){ cnt++; }
    ~this(){ cnt--; }
}

void main()
{
    {
    B b = B(42);
    }
    assert(B.cnt == 0);//passes
    assert(A.cnt == 0);//fails A.cnt == -1
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #2 from Dmitry Olshansky <dmitry.olsh gmail.com> 2011-09-01
13:58:35 PDT ---
It might be more complicated then I thought, postblits of members do work.
I'd better leave the cause of problem to thouse in the know. Another variation
of test:

import std.stdio;

struct A
{
    static int ctor, post, dtor;

    this(int dummy){ ctor++; }
    this(this){ post++; }
    ~this(){ dtor++; }
}

struct B
{
    A a;
    static int ctor, post, dtor;
    this(int dummy){ 
        a = A(dummy); // a(dummy) was a typo, thought it changes nothing
        ctor++; 
    }
    this(this){ post++; }
    ~this(){ dtor++; }
}


void main()
{
    {
        B b = B(42);
        auto c = b;
    }
        // all works as long as it's "shallow"
    assert(B.post == 1);
    assert(B.ctor == 1);
    assert(B.dtor == 2);

    writefln("%s %s %s", A.ctor, A.post, A.dtor);//prints 1 1 3
    assert(A.ctor == 1);
    assert(A.post == 1);
    assert(A.dtor == 2);//fails
}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 01 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #3 from Kenji Hara <k.hara.pg gmail.com> 2011-09-20 07:07:18 PDT ---
(In reply to comment #2)

 It might be more complicated then I thought, postblits of members do work.
 I'd better leave the cause of problem to thouse in the know. Another variation
 of test:


I think this is right behavior, and no problem.

Please note this line:

         a = A(dummy); // a(dummy) was a typo, thought it changes nothing


This is "assignment", not initializing.
The assignment of an object that has postblit (like A) is implemented *swap and
destroy*. For this purpose, D compiler implements opAssign implicitly, like
follows:

struct A {
  ...
  ref A opAssign(A rhs) {  // rhs is copyed from original value
    std.algorithm.swap(this, rhs);  // bitwise swapping
    return rhs;
    // rhs is equals to original 'this', and it is destroyed here.
  }
}

Therefore, the assignment of an object of A always increment A.dtor.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #4 from Dmitry Olshansky <dmitry.olsh gmail.com> 2011-09-20
07:37:10 PDT ---

 Please note this line:

         a = A(dummy); // a(dummy) was a typo, thought it changes nothing


 
 This is "assignment", not initializing.
 The assignment of an object that has postblit (like A) is implemented *swap and
 destroy*. 


Note this line was in constructor. No way to initialize member of a struct?
That's something I'd call unacceptable.

I should point out that move == swap & destroy, iff left side of assigment
_was_ initialized. A constructor may be called on chunk of uninitialized memory
e.g. in Phobos std.typecons.emplace.


 For this purpose, D compiler implements opAssign implicitly, like
 follows:
 
 struct A {
   ...
   ref A opAssign(A rhs) {  // rhs is copyed from original value


why do we copying the original value in the first place? 
It should be moved with e.g. memmov


     std.algorithm.swap(this, rhs);  // bitwise swapping
     return rhs;
     // rhs is equals to original 'this', and it is destroyed here.
   }
 }




 
 Therefore, the assignment of an object of A always increment A.dtor.


Thanks, that clarifies it in part, but still how about initialization in
constructor?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #5 from Kenji Hara <k.hara.pg gmail.com> 2011-09-20 08:07:19 PDT ---
(In reply to comment #4)

 Please note this line:

         a = A(dummy); // a(dummy) was a typo, thought it changes nothing


 
 This is "assignment", not initializing.
 The assignment of an object that has postblit (like A) is implemented *swap and
 destroy*. 


 
 Note this line was in constructor. No way to initialize member of a struct?
 That's something I'd call unacceptable.
 
 I should point out that move == swap & destroy, iff left side of assigment
 _was_ initialized. A constructor may be called on chunk of uninitialized memory
 e.g. in Phobos std.typecons.emplace.


In B's constructor, the member a is already intialized by A.init. So `a =
A(dummy);` is always assignment.

And, yes, I think using emplace is right way to *initialize* member a.

emplace(&a, dummy);  // a is treated as an uninitialized memory

But, unfortunately, emplace has a bug. This does not work as our expected.



 why do we copying the original value in the first place? 
 It should be moved with e.g. memmov


Ah... my explanation had a bit misleading. When opAssign receives rvalue, rhs
is just moved. Otherwise, rhs is coped. In this case, A(dummy) is treated as
rvalue, so it is moved.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #6 from Dmitry Olshansky <dmitry.olsh gmail.com> 2011-09-20
13:01:11 PDT ---

 I should point out that move == swap & destroy, iff left side of assigment
 _was_ initialized. A constructor may be called on chunk of uninitialized memory
 e.g. in Phobos std.typecons.emplace.


 
 In B's constructor, the member a is already intialized by A.init. So `a =
 A(dummy);` is always assignment.


Example:
ubyte[B.sizeof] mem=void;
emplace!B(mem.ptr);//Does this call to B's constructor call A's dtor on some
kind of trash then?


 
 And, yes, I think using emplace is right way to *initialize* member a.
 


And that's a problem. I mean even when emplace is working and all. Do we really
want everybody to write emplace(&a, dummy); to do initialization in
constructor?
Seems very backwards.
I'd hate it if this will be some kind of rule #22 of how to do things correctly
in D.



 emplace(&a, dummy);  // a is treated as an uninitialized memory
 
 But, unfortunately, emplace has a bug. This does not work as our expected.
 
 

 why do we copying the original value in the first place? 
 It should be moved with e.g. memmov


 
 Ah... my explanation had a bit misleading. When opAssign receives rvalue, rhs
 is just moved. Otherwise, rhs is coped. In this case, A(dummy) is treated as
 rvalue, so it is moved.


Ok, glad it works this way.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #7 from Kenji Hara <k.hara.pg gmail.com> 2011-09-20 13:39:38 PDT ---
(In reply to comment #6)

 Example:
 ubyte[B.sizeof] mem=void;
 emplace!B(mem.ptr);//Does this call to B's constructor call A's dtor on some
 kind of trash then?


void main()
{
    ubyte[B.sizeof] mem=void;
    emplace!B(cast(void[])mem[]);
    writefln("%s %s %s", A.ctor, A.post, A.dtor);//prints 0 0 1
    writefln("%s %s %s", B.ctor, B.post, B.dtor);//prints 0 0 1
    // emplace calls A's ctor through calling B's ctor.
}


 And that's a problem. I mean even when emplace is working and all. Do we really
 want everybody to write emplace(&a, dummy); to do initialization in
 constructor?
 Seems very backwards.
 I'd hate it if this will be some kind of rule #22 of how to do things correctly
 in D.


I think the cases that actually needs emplace is rare.
In most cases, it is rare that T.init has a meaningful state. (In this context,
'meaningful' means calling destructor against T.init occurs something.)



 
 

 emplace(&a, dummy);  // a is treated as an uninitialized memory
 
 But, unfortunately, emplace has a bug. This does not work as our expected.
 
 

 why do we copying the original value in the first place? 
 It should be moved with e.g. memmov


 
 Ah... my explanation had a bit misleading. When opAssign receives rvalue, rhs
 is just moved. Otherwise, rhs is coped. In this case, A(dummy) is treated as
 rvalue, so it is moved.


 
 Ok, glad it works this way.


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2011
prev sibling next sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581



--- Comment #8 from Kenji Hara <k.hara.pg gmail.com> 2011-09-20 13:50:24 PDT ---
Sorry, that was imcomplete.

(In reply to comment #6)

 Example:
 ubyte[B.sizeof] mem=void;
 emplace!B(mem.ptr);//Does this call to B's constructor call A's dtor on some
 kind of trash then?


void main()
{
    ubyte[B.sizeof] mem=void;
    emplace!B(cast(void[])mem[]);
    writefln("%s %s %s", A.ctor, A.post, A.dtor);//prints 0 0 1
    writefln("%s %s %s", B.ctor, B.post, B.dtor);//prints 0 0 1
    // emplace calls A's ctor through calling B's ctor.
}


 And that's a problem. I mean even when emplace is working and all. Do we really
 want everybody to write emplace(&a, dummy); to do initialization in
 constructor?
 Seems very backwards.
 I'd hate it if this will be some kind of rule #22 of how to do things correctly
 in D.


I think the cases that actually needs emplace is rare.
In most cases, it is rare that T.init has a meaningful state.

(In this context, 'meaningful' means calling destructor against T.init does
something.
e.g. reference counter == 0, class reference == null, ...)

Therefore, destructor calling with assignment against T.init like `a =
A(dummy)` does not make problems usually.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2011
prev sibling parent d-bugmail puremagic.com writes:
http://d.puremagic.com/issues/show_bug.cgi?id=6581


Dmitry Olshansky <dmitry.olsh gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |INVALID


--- Comment #9 from Dmitry Olshansky <dmitry.olsh gmail.com> 2011-09-20
14:05:32 PDT ---
(In reply to comment #8)

 Sorry, that was imcomplete.
 
 (In reply to comment #6)

 Example:
 ubyte[B.sizeof] mem=void;
 emplace!B(mem.ptr);//Does this call to B's constructor call A's dtor on some
 kind of trash then?


 
 void main()
 {
     ubyte[B.sizeof] mem=void;
     emplace!B(cast(void[])mem[]);
     writefln("%s %s %s", A.ctor, A.post, A.dtor);//prints 0 0 1
     writefln("%s %s %s", B.ctor, B.post, B.dtor);//prints 0 0 1
     // emplace calls A's ctor through calling B's ctor.
 }
 


This doesn't call constructor at all it copies B.init over mem, this one will
do:

void main()
{
    ubyte[B.sizeof] mem=void;
    emplace!B(cast(void[])mem[], 33);
    writefln("%s %s %s", A.ctor, A.post, A.dtor);//prints 1 0 2
    writefln("%s %s %s", B.ctor, B.post, B.dtor);//prints 1 0 1
}
The interesting moment is that emplace first does overwrite memory with T.init
so it seems like there is no way to call constructor on a raw memory. (at least
 not untill you use __ctor on your own risk)


 And that's a problem. I mean even when emplace is working and all. Do we really
 want everybody to write emplace(&a, dummy); to do initialization in
 constructor?
 Seems very backwards.
 I'd hate it if this will be some kind of rule #22 of how to do things correctly
 in D.


 
 I think the cases that actually needs emplace is rare.
 In most cases, it is rare that T.init has a meaningful state.
 
 (In this context, 'meaningful' means calling destructor against T.init does
 something.
 e.g. reference counter == 0, class reference == null, ...)
 
 Therefore, destructor calling with assignment against T.init like `a =
 A(dummy)` does not make problems usually.


Yes, but I was thinking about cases where destructor could be called on raw
memory causes things like: free(some_random_address); 
I just was investigating a crush and the cause looked like memory freed two
times in two calls to destructor, looks like I'm off on this one, though.
I'm closing it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
Sep 20 2011