www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - struct dtor never called, what's wrong?

reply Andrea Fontana <nospam example.com> writes:
Maybe it's just a big blunder, but I think there's something 
wrong in phobos.

All started a couple of days ago, when I was investigating on 
some not-freed resource on my code.

It turns out that on "each" function inside phobos it is used a 
"ref foreach". Something like:

foreach(ref element, myRange) {}

If myRange.front() doesn't return a ref, this syntax probably 
should be rejected, but it is accepted. So for each iterated 
element a ctor is called but not a dtor.

Discussing this with Rikki we discovered a similar bug is still 
opened [1] but was not related with missing dtor call, actually.

If "each" maybe is not so used, a similar problem popped out in 
another part of my code, using std.array : array. Apparently 
there is no "for-each-ref" code there.

That's fun because I was actually working on a workaround for 
another bug [2] I've found in the same code base.

Anyway code [3] (assumePure is a workaround for [2]) shows that 4 
dtor call are missing (element constructed by array).

I wonder how diffused is this bug considering that "array" is a 
pretty used function. This leads to a leak on my code.

I hope I'm wrong. Maybe I'm missing something. Anyone can help?

Andrea

[1] https://issues.dlang.org/show_bug.cgi?id=11934
[2] 
https://forum.dlang.org/post/kfaakznkdnnoxwcrgtsg forum.dlang.org
[3] https://run.dlang.io/is/IMqYzK
Jun 15 2018
parent reply ketmar <ketmar ketmar.no-ip.org> writes:
Andrea Fontana wrote:


 I hope I'm wrong. Maybe I'm missing something. Anyone can help?


you are not wrong, this is bug in DMDFE.
Jun 15 2018
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 6/15/18 10:51 AM, ketmar wrote:

 Andrea Fontana wrote:
 

 I hope I'm wrong. Maybe I'm missing something. Anyone can help?


 
 you are not wrong, this is bug in DMDFE.


Yep.

I tried using -vcg-ast, and see an interesting lowering:

for (; !__r115.empty(); __r115.popFront())
{
     ref A r = __r115.front();
...

I believe that what happens is r really is a ref to... a temporary 
return. Because the compiler at this point won't re-allocate that memory 
to something else, it works, but this is actually a memory bug too!

The destructor likely is not added, because the AST doesn't show any 
variable that needs it! r is a ref.

At the very least, the code should allocate a temporary and assign a ref 
to that, so lifetime management works properly. I agree the better path 
is to simply disallow ref, but this would be a non-code-breaking step 
that would fix the issue of lifetime management.

Note that in D1, only opApply was the way to use foreach, and it 
*requires* ref for the element, even if your foreach didn't use ref (and 
even if the actual thing you are iterating didn't make sense as ref). It 
was actually impossible to duplicate array foreach, which would not 
allow you to ref the index. You'd have to create a temporary index, send 
it into the delegate, and ignore any changes.

The lowering for ranges must have tried to simulate this ability 
(allowing you to use ref or non-ref despite what the actual range 
provides), but this kludge doesn't seem to be implemented properly.

-Steve
Jun 15 2018
parent reply Andrea Fontana <nospam example.com> writes:
On Friday, 15 June 2018 at 15:10:54 UTC, Steven Schveighoffer 
wrote:

 I tried using -vcg-ast, and see an interesting lowering:

 for (; !__r115.empty(); __r115.popFront())
 {
     ref A r = __r115.front();
 ...


Also for std.array:array?

I don't get where it is using foreach with refs.
Can be this [1] the problem instead?

[1] https://github.com/dlang/phobos/blob/master/std/array.d#L126

Andrea
Jun 15 2018
parent reply Steven Schveighoffer <schveiguy yahoo.com> writes:
On 6/15/18 11:18 AM, Andrea Fontana wrote:

 On Friday, 15 June 2018 at 15:10:54 UTC, Steven Schveighoffer wrote:

 I tried using -vcg-ast, and see an interesting lowering:

 for (; !__r115.empty(); __r115.popFront())
 {
     ref A r = __r115.front();
 ...


 
 Also for std.array:array?
 
 I don't get where it is using foreach with refs.
 Can be this [1] the problem instead?
 
 [1] https://github.com/dlang/phobos/blob/master/std/array.d#L126
 
 Andrea


That is a separate bug. The array function appears not to set the 
correct bits for the GC block. It should set the appendable bit, and the 
finalize and struct finalize bits. All it sets is the no-scan bit. It 
also needs to set up the allocated size properly. I think 
uninitializedArray is a bad idea for array to use when the type being 
inserted has a dtor.

So the code is executing all the proper postblits and destructors, it's 
just that the GC is not executing the destructors.

Can you file a bug or search for one on this? I will see if I can fix it.

-Steve
Jun 15 2018
parent Andrea Fontana <nospam example.com> writes:
On Friday, 15 June 2018 at 15:49:01 UTC, Steven Schveighoffer 
wrote:

 Can you file a bug or search for one on this? I will see if I 
 can fix it.

 -Steve


https://issues.dlang.org/show_bug.cgi?id=18995
Jun 15 2018