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.

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/rt/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?

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

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

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

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

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?

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!

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/rt/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!