www.digitalmars.com         C & C++   DMDScript  

digitalmars.D - delete and __delete not equivalent

reply Imperatorn <johan_forsberg_86 hotmail.com> writes:
https://issues.dlang.org/show_bug.cgi?id=21550
Jan 14 2021
parent reply Nick Treleaven <nick geany.org> writes:
On Friday, 15 January 2021 at 01:28:55 UTC, Imperatorn wrote:

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


 From that issue, it's surprising that delete frees array memory 
even when the ptr field isn't the start of the allocation. I 
would've guessed calling a GC function to look up the start of 
the allocation would be needed first. That function might be 
useful in other contexts too.
Jan 15 2021
next sibling parent Imperatorn <johan_forsberg_86 hotmail.com> writes:
On Friday, 15 January 2021 at 13:15:12 UTC, Nick Treleaven wrote:

 On Friday, 15 January 2021 at 01:28:55 UTC, Imperatorn wrote:

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


 From that issue, it's surprising that delete frees array memory 
 even when the ptr field isn't the start of the allocation. I 
 would've guessed calling a GC function to look up the start of 
 the allocation would be needed first. That function might be 
 useful in other contexts too.


Yeah, I wonder how many use this new approach. They would suffer 
in silence 🤐

Should be an easy fix tho. Could maybe get it into the next 
upcoming release even.
Jan 15 2021
prev sibling parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Friday, 15 January 2021 at 13:15:12 UTC, Nick Treleaven wrote:

 From that issue, it's surprising that delete frees array memory 
 even when the ptr field isn't the start of the allocation.


while(true) {
   int[] a = new int[](whatever);
   delete a;
}

That will not run out of memory. Change it to `__delete(a);` or 
`GC.free(a.ptr)` like the docs tell you to do and it will.

It is true that if you slice a and delete it, it still blasts the 
whole block which might be surprising.... but like plain simple 
new/delete pair actually working certainly isn't surprising and 
plain simple new/__delete pair doing absolutely nothing makes the 
whole thing pretty worthless. Hence the bug report.
Jan 15 2021
parent reply Steven Schveighoffer <schveiguy gmail.com> writes:
On 1/15/21 8:59 AM, Adam D. Ruppe wrote:

 On Friday, 15 January 2021 at 13:15:12 UTC, Nick Treleaven wrote:

 From that issue, it's surprising that delete frees array memory even 
 when the ptr field isn't the start of the allocation.


 
 while(true) {
    int[] a = new int[](whatever);
    delete a;
 }
 
 That will not run out of memory. Change it to `__delete(a);` or 
 `GC.free(a.ptr)` like the docs tell you to do and it will.


To clarify, as long as whatever is larger than 2k, it will. And that's a 
bug for sure (the reason for this is based on the GC implementation).


 It is true that if you slice a and delete it, it still blasts the whole 
 block which might be surprising.... but like plain simple new/delete 
 pair actually working certainly isn't surprising and plain simple 
 new/__delete pair doing absolutely nothing makes the whole thing pretty 
 worthless. Hence the bug report.


I'm unsure if deleting from the middle is expected or desired.

I would first fix it to do what the delete call does, and then possibly 
"fix" that behavior, as I wouldn't expect it either. If you pass a slice 
into a function, that function probably shouldn't affect everything in 
the block from that slice.

But clearly a bug in the first respect.

-Steve
Jan 15 2021
parent reply Adam D. Ruppe <destructionator gmail.com> writes:
On Friday, 15 January 2021 at 14:38:55 UTC, Steven Schveighoffer 
wrote:

 I'm unsure if deleting from the middle is expected or desired.


Yeah, in my private implementation, I have a sanity check that 
the pointer and the base aren't too far apart. I figured they 
didn't line up due to a capacity cache or similar metadata at the 
beginning of the block (though I didn't prove this) and thus the 
pointers shouldn't be more than like 16 bytes apart and anything 
more than that is suspicious.

So far in my tests that appears to be exactly the case - 16 bytes 
on both 32 and 64 bit.

But while that might be good enough for my barely-documented 
"WARNING: calling this function might break your code" thing, idk 
if that is the right answer for druntime itself.

However if I can prove it is correct, I do think it is reasonable 
to go back to silently doing nothing if the pointers are further 
apart since freeing the whole block from a partial slice is a bit 
iffy in terms of correctness. You wouldn't do that with malloc.
Jan 15 2021
parent Imperatorn <johan_forsberg_86 hotmail.com> writes:
On Saturday, 16 January 2021 at 04:52:05 UTC, Adam D. Ruppe wrote:

 On Friday, 15 January 2021 at 14:38:55 UTC, Steven 
 Schveighoffer wrote:

 [...]


 Yeah, in my private implementation, I have a sanity check that 
 the pointer and the base aren't too far apart. I figured they 
 didn't line up due to a capacity cache or similar metadata at 
 the beginning of the block (though I didn't prove this) and 
 thus the pointers shouldn't be more than like 16 bytes apart 
 and anything more than that is suspicious.

 [...]


Any way, this should be included in the next release imo.

It doesn't look good for D to have memory leaks when using the 
recommended alternative to delete...
Jan 16 2021