Bug 21550 – core.memory.__delete does not actually work

I just had a user come to me perplexed that a big array wasn't being freed. Since they on 32 bit working with large images, I figured it was a false pointer (and no, the semi-precise GC doesn't help either - we tried - cuz stack pointers can still pin it). But that's OK, I said, there's a public, just undocumented, clear function you can use which frees it manually. The memory leak persisted. I looked into it and, nope, my library frees everything it mallocs, and the clear function calls __delete, just like the documentation says to do ever since the delete keyword was deprecated. But indeed, the memory leak persists. Replacing it with the original delete keyword works fine. After looking closer, I see the problem: https://dlang.org/phobos/core_memory.html#.__delete is broken. See, the original _d_delarrayT function does this: GC.free(info.base); The new __delete implementation does this: GC.free(cast(void*) x.ptr). The difference: http://dpldocs.info/experimental-docs/core.memory.GC.free.html "if p points to the interior of a memory block [...] no action will be taken." An array is not actually necessarily at the start of a block. Making __delete (or a direct call to GC.free for that matter) a silent no-op.

BTW also this page: https://dlang.org/deprecate.html#delete says to call GC.free instead. This is incorrect as GC.free(arr.ptr) is a do-nothing operation. You MUST get the base of the block. I'm slightly concerned that the block might be shared with some other array.... is there ever a case where two small `new` allocated arrays would ever share a GC block? Of course I know other slices can use it, but that's expected, I'm just concerned about surprising action at a distance. If that never happens, just doing GC.free(GC.addrOf(ptr)) I believe is an acceptable fix in both __Delete's impl and in that documentation page's suggestion.

To clarify, a new array will be pointing at the front of the block unless the array size is large enough to be in an extendable block. Last I checked, that was 2k bytes. The reason is simple: array size is stored at the end of small blocks for alignment. But an extendable block can have the block size change from one call to the next. To avoid issues with that happening, the array size is stored at the front of the block, pushing the array off by some number of bytes. This leads to arr.ptr not pointing at the front of the block. The fix is also simple -- use the info.base just like the original.

@adamdruppe created dlang/druntime pull request #3481 "Fix issue 21550" fixing this issue: - Fix issue 21550 https://github.com/dlang/druntime/pull/3481

dlang/druntime pull request #3481 "Fix issue 21550" was merged into master: - 471cbd14c11c15be31b48373cd794f94656b439a by Adam D. Ruppe: Fix issue 21550 https://github.com/dlang/druntime/pull/3481