Bug 6581 – Yet another dtor/postblit problem?

Status
RESOLVED
Resolution
INVALID
Severity
major
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
Other
OS
All
Creation time
2011-08-30T14:12:00Z
Last change time
2011-09-20T14:05:32Z
Assigned to
nobody
Creator
dmitry.olsh

Comments

Comment #0 by dmitry.olsh — 2011-08-30T14:12:40Z
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)
Comment #1 by dmitry.olsh — 2011-09-01T13:21:38Z
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 }
Comment #2 by dmitry.olsh — 2011-09-01T13:58:35Z
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 }
Comment #3 by k.hara.pg — 2011-09-20T07:07:18Z
(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.
Comment #4 by dmitry.olsh — 2011-09-20T07:37:10Z
> 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?
Comment #5 by k.hara.pg — 2011-09-20T08:07:19Z
(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.
Comment #6 by dmitry.olsh — 2011-09-20T13:01:11Z
> > 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.
Comment #7 by k.hara.pg — 2011-09-20T13:39:38Z
(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.
Comment #8 by k.hara.pg — 2011-09-20T13:50:24Z
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.
Comment #9 by dmitry.olsh — 2011-09-20T14:05:32Z
(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.