www.digitalmars.com         C & C++   DMDScript  

digitalmars.D.learn - Arrays of noncopyables/Invalid memory operation

reply Matt Elkins <notreal fake.com> writes:
So in a different thread someone mentioned that when arrays are 
grown an implicit copy could be called on all the elements, as 
they might need to be copied over to a new, larger block of 
memory. This makes sense, and is expected. However, it got me 
concerned: what if the post-blit was disabled because it is 
illegal to copy the object? I am using a lot of objects exactly 
like this, and wanted to make sure my program wasn't working by 
coincidence since my dynamic arrays are still small for now. So I 
tested:

[code]
import std.stdio;
 safe:

bool scopeEnded;

struct Foo
{
      disable this(this);

     this(int val) {writeln("Constructing: ", val, " (", &this, 
")"); value = val;}
     ~this() {writeln("Destroying: ", value, " (", &this, ")"); 
assert(value == int.init || scopeEnded);}
     int value;
}

unittest
{
     Foo[] foos;
     for (auto i = 0; i < 10000; ++i)
     {
         ++foos.length;
         foos[$ - 1] = Foo(i);
     }

     writeln("Scope about to end");
     scopeEnded = true;
}
[/code]

[output]
Constructing: 0 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 1 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 2 (18FDA8)
Destroying: 0 (18FD6C)

....<snipped>....

Constructing: 410 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 411 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 412 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 413 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 414 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 415 (18FDA8)

Destroying: 0 (18FD6C)
core.exception.InvalidMemoryOperationError src\core\exception.d(679): Invalid
memory operation
Constructing: 416 (18FDA8)
----------------
Destroying: 0 (18FD6C)

Constructing: 417 (18FDA8)
core.exception.InvalidMemoryOperationError src\core\exception.d(679): Invalid
memory operation
Destroying: 0 (18FD6C)
Constructing: 418 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 419 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 420 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 421 (18FDA8)
----------------
Destroying: 0 (18FD6C)
Constructing: 422 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 423 (18FDA8)

....<snipped>....

Constructing: 506 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 507 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 508 (18FDA8)
Destroying: 0 (18FD6C)
Constructing: 509 (18FDA8)
Destroying: 0 (18FD6C)
Destroying: 29 (5201F4)
Program exited with code 1
[/output]

Note that the invalid memory operation lines change relative 
order in this output, I think maybe it is stderr instead of 
stdout.

So now I'm wondering:
* Is the fact that this compiles a bug? If copying can happen 
under the hood, shouldn't the  disable this(this) prevent Foo 
being used this way? Or should copying be happening at all (the 
compiler could instead choose to "move" the Foo by blitting it 
and NOT running the destructor...though I don't know whether that 
is a safe choice in the general case)?
* What is the invalid memory operation? It doesn't occur if I 
remove the assert or make the assert never fail, so I'm guessing 
it has to do with failing an assert in the destructor? This 
points bothers me less than the previous one because I don't 
expect an assert-failing program to behave nicely anyway.
Feb 17 2016
parent reply ZombineDev <valid_email he.re> writes:
On Wednesday, 17 February 2016 at 22:20:00 UTC, Matt Elkins wrote:

 So in a different thread someone mentioned that when arrays are 
 grown an implicit copy could be called on all the elements, as 
 they might need to be copied over to a new, larger block of 
 memory. This makes sense, and is expected. However, it got me 
 concerned: what if the post-blit was disabled because it is 
 illegal to copy the object? I am using a lot of objects exactly 
 like this, and wanted to make sure my program wasn't working by 
 coincidence since my dynamic arrays are still small for now. So 
 I tested:

 [code]
 import std.stdio;
  safe:

 bool scopeEnded;

 struct Foo
 {
      disable this(this);

     this(int val) {writeln("Constructing: ", val, " (", &this, 
 ")"); value = val;}
     ~this() {writeln("Destroying: ", value, " (", &this, ")"); 
 assert(value == int.init || scopeEnded);}
     int value;
 }

 unittest
 {
     Foo[] foos;
     for (auto i = 0; i < 10000; ++i)
     {
         ++foos.length;
         foos[$ - 1] = Foo(i);
     }

     writeln("Scope about to end");
     scopeEnded = true;
 }
 [/code]

 [output]
 Constructing: 0 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 1 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 2 (18FDA8)
 Destroying: 0 (18FD6C)

 ....<snipped>....

 Constructing: 410 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 411 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 412 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 413 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 414 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 415 (18FDA8)

 Destroying: 0 (18FD6C)
 core.exception.InvalidMemoryOperationError src\core\exception.d(679): Invalid
memory operation
 Constructing: 416 (18FDA8)
 ----------------
 Destroying: 0 (18FD6C)

 Constructing: 417 (18FDA8)
 core.exception.InvalidMemoryOperationError src\core\exception.d(679): Invalid
memory operation
 Destroying: 0 (18FD6C)
 Constructing: 418 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 419 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 420 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 421 (18FDA8)
 ----------------
 Destroying: 0 (18FD6C)
 Constructing: 422 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 423 (18FDA8)

 ....<snipped>....

 Constructing: 506 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 507 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 508 (18FDA8)
 Destroying: 0 (18FD6C)
 Constructing: 509 (18FDA8)
 Destroying: 0 (18FD6C)
 Destroying: 29 (5201F4)
 Program exited with code 1
 [/output]

 Note that the invalid memory operation lines change relative 
 order in this output, I think maybe it is stderr instead of 
 stdout.

 So now I'm wondering:
 * Is the fact that this compiles a bug? If copying can happen 
 under the hood, shouldn't the  disable this(this) prevent Foo 
 being used this way? Or should copying be happening at all (the 
 compiler could instead choose to "move" the Foo by blitting it 
 and NOT running the destructor...though I don't know whether 
 that is a safe choice in the general case)?


 From looking at the addresses of the objects it seems like 
they're being constructed on the stack (as a temporary object), 
copied (blitted) to the array on the heap and then the temporary 
object has it's destructor being run. I think this is safe 
because you can't access the temporary (only the compiler can see 
it), so it's uniqueness is preserved. Unnecessarily running 
destructor is either an optimization opportunity or a 
manifestation of the bug that you reported here: 
https://issues.dlang.org/show_bug.cgi?id=15661. I suggest testing 
this code again with a newer compiler (see my answer in the other 
thread - 
http://forum.dlang.org/post/omfyqfulgyzbrxlzrhvq forum.dlang.org).


 * What is the invalid memory operation? It doesn't occur if I 
 remove the assert or make the assert never fail, so I'm 
 guessing it has to do with failing an assert in the destructor? 
 This points bothers me less than the previous one because I 
 don't expect an assert-failing program to behave nicely anyway.


The "Invalid memory operation" error is thrown only by the GC 
(AFAIK) when the user tries something unsupported like allocating 
or freeing in a destructor, while a GC collection is being run. I 
think that it's not possible to trigger it in  nogc code.
 From a short debug session, I managed to find out that it crashes 
on the `++foos.length` line, somewhere inside this function: 
https://github.com/D-Programming-Language/druntime/blob/e47a00bff935c3f079bb567a6ec97663ba384487/src/
t/lifetime.d#L1265. Unfortunately I currently don't have enough time to
investigate further.
Can you please file a bugzilla issue with this test case?
Feb 17 2016
next sibling parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 02/17/2016 05:14 PM, ZombineDev wrote:


 The "Invalid memory operation" error is thrown only by the GC (AFAIK)
 when the user tries something unsupported like allocating or freeing in
 a destructor, while a GC collection is being run.


That. The problem is when the assert check fails (for value==60). The GC 
fails to create the exception object.

Ali
Feb 17 2016
parent reply ZombineDev <valid_email he.re> writes:
On Thursday, 18 February 2016 at 01:19:16 UTC, Ali Çehreli wrote:

 On 02/17/2016 05:14 PM, ZombineDev wrote:


 The "Invalid memory operation" error is thrown only by the GC


 (AFAIK)

 when the user tries something unsupported like allocating or


 freeing in

 a destructor, while a GC collection is being run.


 That. The problem is when the assert check fails (for 
 value==60). The GC fails to create the exception object.

 Ali


Hmm, why does the destructor of object 60 run in the middle of 
nowhere? I remember seeing it crash while my debugger was in 
_d_arraysetlengthT [1], though I don't know exactly where because 
I didn't have debug symbols for druntime. __doPostblit[2] checks 
if the type has disabled this(this) and if so it doesn't do 
anything. So the destructor should not have been run because of 
the array resize. Seems like there maybe some memory corruption 
or a bug in druntime.

[1]: 
https://github.com/D-Programming-Language/druntime/blob/e47a00bff935c3f079bb567a6ec97663ba384487/src/rt/lifetime.d#L1265

[2]: 
https://github.com/D-Programming-Language/druntime/blob/e47a00bff935c3f079bb567a6ec97663ba384487/src/rt/lifetime.d#L566
Feb 17 2016
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 02/17/2016 07:00 PM, ZombineDev wrote:

 On Thursday, 18 February 2016 at 01:19:16 UTC, Ali Çehreli wrote:

 On 02/17/2016 05:14 PM, ZombineDev wrote:


 The "Invalid memory operation" error is thrown only by the GC


 (AFAIK)

 when the user tries something unsupported like allocating or


 freeing in

 a destructor, while a GC collection is being run.


 That. The problem is when the assert check fails (for value==60). The
 GC fails to create the exception object.




I found an open issue that depends on changing the GC implementation:

   https://issues.dlang.org/show_bug.cgi?id=7349


 Hmm, why does the destructor of object 60 run in the middle of nowhere?


It so happens that the GC decides to collect when moving the array to a 
new location at that point. If you add the following to the beginning of 
main, you will see that the next object to destroy is 59, then 58, etc.

     import core.memory;
     GC.disable;

So, everything makes sense other than throwing in a destructor having 
issues with the current GC. :-/

Ali
Feb 18 2016
parent reply =?UTF-8?Q?Ali_=c3=87ehreli?= <acehreli yahoo.com> writes:
On 02/18/2016 12:12 PM, Ali Çehreli wrote:


  > Hmm, why does the destructor of object 60 run in the middle of 


nowhere?

 It so happens that the GC decides to collect when moving the array to a
 new location at that point. If you add the following to the beginning of
 main, you will see that the next object to destroy is 59, then 58, etc.

      import core.memory;
      GC.disable;

 So, everything makes sense other than throwing in a destructor having
 issues with the current GC. :-/


No, it doesn't make sense to me anymore.

All we're doing here is growing an array. There shouldn't be any 
destructor calls when just doing that, no? As the array grows, the 
elements should be "moved" to the new location; why the destructor? I 
hope this is related to the recent fix.

Ali
Feb 18 2016
parent reply "H. S. Teoh via Digitalmars-d-learn" <digitalmars-d-learn puremagic.com> writes:
On Thu, Feb 18, 2016 at 05:18:33PM -0800, Ali Çehreli via Digitalmars-d-learn
wrote:

 On 02/18/2016 12:12 PM, Ali Çehreli wrote:
 

  > Hmm, why does the destructor of object 60 run in the middle of
  > nowhere?

 It so happens that the GC decides to collect when moving the array
 to a new location at that point. If you add the following to the
 beginning of main, you will see that the next object to destroy is
 59, then 58, etc.

      import core.memory;
      GC.disable;

 So, everything makes sense other than throwing in a destructor
 having issues with the current GC. :-/


 
 No, it doesn't make sense to me anymore.
 
 All we're doing here is growing an array. There shouldn't be any
 destructor calls when just doing that, no? As the array grows, the
 elements should be "moved" to the new location; why the destructor? I
 hope this is related to the recent fix.


[...]

Consider this:

	struct HasDtor {
		int data;
		~this() { ... }
	}
	HasDtor[] func(HasDtor[] arr) {
		HasDtor[] middleSlice;
		for (i; 0 .. 1000) {
			arr ~= HasDtor(i);
			if (i == 500) {
				middleSlice = arr;
			}
		}
		return arr;
	}

Suppose the array gets moved sometime after i=500 because it ran out of
space in the current memory location.  Since there is another slice
middleSlice pointing at the old data, it's not just a matter of *moving*
the elements over to the new location; they have to be *copied* over so
that there are now two copies of the original elements -- the GC doesn't
know whether func may try to access the original elements through
middleSlice, so it cannot just move them. Only when middleSlice goes out
of scope, can the old elements be destructed.

So we see that when an array is grown, the elements cannot simply be
moved to the new location; we must make copies of them, otherwise any
slice of the old data will become invalid. So after an array is moved,
there will be two copies of data, and if the original elements are
unreferenced after all, the GC will call the dtors when it runs the next
collection cycle.  Then when the new elements become unreferenced, the
dtors will be called again at the following collection cycle, on the new
copies of the elements.


T

-- 
It always amuses me that Windows has a Safe Mode during bootup. Does that mean
that Windows is normally unsafe?
Feb 18 2016
parent Matt Elkins <notreal fake.com> writes:
On Friday, 19 February 2016 at 01:30:13 UTC, H. S. Teoh wrote:

 Suppose the array gets moved sometime after i=500 because it 
 ran out of space in the current memory location.  Since there 
 is another slice middleSlice pointing at the old data, it's not 
 just a matter of *moving* the elements over to the new 
 location; they have to be *copied* over so that there are now 
 two copies of the original elements -- the GC doesn't know 
 whether func may try to access the original elements through 
 middleSlice, so it cannot just move them. Only when middleSlice 
 goes out of scope, can the old elements be destructed.

 So we see that when an array is grown, the elements cannot 
 simply be moved to the new location; we must make copies of 
 them, otherwise any slice of the old data will become invalid. 
 So after an array is moved, there will be two copies of data, 
 and if the original elements are unreferenced after all, the GC 
 will call the dtors when it runs the next collection cycle.  
 Then when the new elements become unreferenced, the dtors will 
 be called again at the following collection cycle, on the new 
 copies of the elements.


So it seems that the conclusion is that it is indeed unsafe to 
store non-copyable objects in an array which might grow (though 
it would be nice if attempting to do so was a compiler error, 
like it is with std.container.Array). I've starting creating a 
custom "Vector" type with value semantics which is move-aware, so 
hopefully that will address my needs.

Thanks for all the help, folks!
Feb 19 2016
prev sibling parent Matt Elkins <notreal fake.com> writes:
On Thursday, 18 February 2016 at 01:14:00 UTC, ZombineDev wrote:

 https://issues.dlang.org/show_bug.cgi?id=15661. I suggest 
 testing this code again with a newer compiler (see my answer in 
 the other thread - 
 http://forum.dlang.org/post/omfyqfulgyzbrxlzrhvq forum.dlang.org).


Thanks -- I might get around to that at some point, but for now 
I'm ok waiting for pre-built binaries unless it becomes more of 
an issue (I've diverted far off what I need to be working on 
already).


 The "Invalid memory operation" error is thrown only by the GC 
 (AFAIK) when the user tries something unsupported like 
 allocating or freeing in a destructor, while a GC collection is 
 being run. I think that it's not possible to trigger it in 
  nogc code.
 From a short debug session, I managed to find out that it 
 crashes on the `++foos.length` line, somewhere inside this 
 function: 
 https://github.com/D-Programming-Language/druntime/blob/e47a00bff935c3f079bb567a6ec97663ba384487/src/
t/lifetime.d#L1265. Unfortunately I currently don't have enough time to
investigate further.
 Can you please file a bugzilla issue with this test case?


Done: https://issues.dlang.org/show_bug.cgi?id=15705

Thanks for the help!
Feb 19 2016