Thread overview
[Issue 23556] Array append ignores copy constructor
Dec 16, 2022
Teodor Dutu
Dec 16, 2022
Paul Backus
Dec 17, 2022
Iain Buclaw
December 14, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

Steven Schveighoffer <schveiguy@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |schveiguy@gmail.com

--
December 14, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

Steven Schveighoffer <schveiguy@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=23557

--
December 14, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

Steven Schveighoffer <schveiguy@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=20879

--
December 14, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

--- Comment #1 from Steven Schveighoffer <schveiguy@gmail.com> ---
Just to give some clearer picture of how this causes issues:

```d
import std.typecons;
import std.sumtype;
import std.stdio;
import core.memory;

void fun(RefCounted!string foo) {
    SumType!( RefCounted!(string) )[] foo_storage;

    foo_storage ~= SumType!( RefCounted!(string) )(foo);
    foo_storage ~= SumType!( RefCounted!(string) )(foo); // reallocates, but no
copy
    foo_storage = null;
}

void clobber()
{
    int[1000] x = 0x123456;
}

void main(){
    RefCounted!( string ) foo = RefCounted!( string )("blah");

    fun(foo);
    clobber();
    GC.collect(); // collect the items already in the GC
    writeln(foo);
}
```

What happens here is:

1. An array of one element is created with the sumtype.
2. The array cannot hold a second element, so it's reallocated. However, the
copy constructor of SumType is *not* called. Therefore, `foo` has a ref count
of 3, but there are afterwards 4 references (the single element array, the
double-element array, and the original foo on the stack).
4. clobber makes sure the stack doesn't point at the arrays.
5. GC.collect cleans up 3 foo elements, reducing the count to 0 and
deallocating.
6. The writeln is now writing garbage. on my system it just spewed random
memory to the screen.

--
December 16, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

Teodor Dutu <teodor.dutu@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |teodor.dutu@gmail.com

--- Comment #2 from Teodor Dutu <teodor.dutu@gmail.com> ---
a ~= S() is not supposed to call either the copy constructor or the postblit
because the newly appended element is supposed to be created in-place inside
the array. This test ensures this behaviour (only the constructor is called,
without the postblit):
https://github.com/dlang/dmd/blob/d405b343e301e5c37a90e66131b3f4d588bed2f2/compiler/test/runnable/sdtor.d#L2244-L2245

The issue comes from copying the array during reallocation. The postblit is correctly called while the cpctor is not. The following code prints "loop" 15 times before printing "pblit" 15 times when the array is reallocated:

---
struct S
{
    this(this)
    {
        writeln("pblit");
    }
}

void main()
{
    S[] a = [ S() ];
    auto p = a.ptr;
    // append until reallocation
    while (a.ptr == p)
    {
        writeln("loop");
        a ~= S(); // no assert
    }
}
---

--
December 16, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

Paul Backus <snarwin+bugzilla@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=23563

--
December 16, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

--- Comment #3 from Steven Schveighoffer <schveiguy@gmail.com> ---
Yes, the problem is the copying of the existing data.

I can think of a few ways to fix this, but they aren't pretty. We either have to add more special members to TypeInfo, or change the hook so it can explain to the runtime how to call the copy constructor.

--
December 17, 2022
https://issues.dlang.org/show_bug.cgi?id=23556

Iain Buclaw <ibuclaw@gdcproject.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Priority|P1                          |P3

--
March 08
https://issues.dlang.org/show_bug.cgi?id=23556

Paul Backus <snarwin+bugzilla@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           See Also|                            |https://issues.dlang.org/sh
                   |                            |ow_bug.cgi?id=24432

--