digitalmars.D - Dynamic array leak?
- bitwise (26/26) Aug 11 2017 struct S {
- Yuxuan Shui (16/42) Aug 11 2017 My guess is a pointer to the array still lives somewhere on the
- bitwise (8/25) Aug 11 2017 Makes sense. I was uncommenting unit tests one-by-one after
- Temtaime (6/34) Aug 12 2017 Collect - is a hit to the GC, not an order. It can ignore this
- bitwise (31/36) Aug 12 2017 If this is the case, then D's GC should have an option to force
- Dgame (6/44) Aug 12 2017 auto s(T, size_t n)(T[n] values)
- bitwise (26/31) Aug 12 2017 This seems to work, but I'm trying to determine if it's 100%
- Steven Schveighoffer (5/6) Aug 14 2017 Hm.. where is this documented? I agree that the GC may not clean up all
- Meta (97/99) Aug 14 2017 As far as I can tell, it is an order. From the GC code[1]:
- Marco Leise (22/23) Aug 12 2017 That can't work and here is why: Druntime employs a
struct S {
static int count = 0;
this(int x) { ++count; }
this(this) { ++count; }
~this() { --count; }
}
int main(string[] argv)
{
S[] x = [S(1), S(1)];
writeln("GC allocated: ", (GC.addrOf(x.ptr) !is null));
x = null;
GC.collect();
writeln("live objects: ", S.count);
return 0;
}
output:
GC allocated: true
live objects: 2
expected:
GC allocated: true
live objects: 0
Is this a bug?
I thought that the first writeln() may be leaving a copy of the
pointer lingering on the stack somewhere, but the output is still
"live objects: 2" with that line commented out.
Thanks
Aug 11 2017
On Friday, 11 August 2017 at 18:44:56 UTC, bitwise wrote:
struct S {
static int count = 0;
this(int x) { ++count; }
this(this) { ++count; }
~this() { --count; }
}
int main(string[] argv)
{
S[] x = [S(1), S(1)];
writeln("GC allocated: ", (GC.addrOf(x.ptr) !is null));
x = null;
GC.collect();
writeln("live objects: ", S.count);
return 0;
}
output:
GC allocated: true
live objects: 2
expected:
GC allocated: true
live objects: 0
Is this a bug?
I thought that the first writeln() may be leaving a copy of the
pointer lingering on the stack somewhere, but the output is
still "live objects: 2" with that line commented out.
Thanks
My guess is a pointer to the array still lives somewhere on the
stack. This gives the expected output:
void f()
{
S[] x = [S(1), S(1)];
writeln("GC allocated: ", (GC.addrOf(x.ptr) !is null));
x = null;
}
int main(string[] argv)
{
f();
GC.collect();
writeln("live objects: ", S.count);
return 0;
}
Aug 11 2017
On Friday, 11 August 2017 at 19:01:44 UTC, Yuxuan Shui wrote:
On Friday, 11 August 2017 at 18:44:56 UTC, bitwise wrote:
[...]
My guess is a pointer to the array still lives somewhere on the
stack. This gives the expected output:
void f()
{
S[] x = [S(1), S(1)];
writeln("GC allocated: ", (GC.addrOf(x.ptr) !is null));
x = null;
}
int main(string[] argv)
{
f();
GC.collect();
writeln("live objects: ", S.count);
return 0;
}
Makes sense. I was uncommenting unit tests one-by-one after
making some changes when I triggered this. I guess they were
passing before because subsequent unit tests cleared the pointers
off the stack. I guess I can just call a function that allocates
a large zeroed-out array on the stack in the last unit test
before checking the count if this happens again.
Thanks
Aug 11 2017
On Friday, 11 August 2017 at 22:36:27 UTC, bitwise wrote:On Friday, 11 August 2017 at 19:01:44 UTC, Yuxuan Shui wrote:Collect - is a hit to the GC, not an order. It can ignore this request. Also do not rely on the gc calling a dtor - it is not safe and can be called totally randomed, so use RC instead or expicit destroy()On Friday, 11 August 2017 at 18:44:56 UTC, bitwise wrote: [...] My guess is a pointer to the array still lives somewhere on the stack. This gives the expected output: void f() { S[] x = [S(1), S(1)]; writeln("GC allocated: ", (GC.addrOf(x.ptr) !is null)); x = null; } int main(string[] argv) { f(); GC.collect(); writeln("live objects: ", S.count); return 0; }Makes sense. I was uncommenting unit tests one-by-one after making some changes when I triggered this. I guess they were passing before because subsequent unit tests cleared the pointers off the stack. I guess I can just call a function that allocates a large zeroed-out array on the stack in the last unit test before checking the count if this happens again. Thanks
Aug 12 2017
On Saturday, 12 August 2017 at 08:16:56 UTC, Temtaime wrote:Collect - is a hint to the GC, not an order. It can ignore this request.If this is the case, then D's GC should have an option to force https://msdn.microsoft.com/en-us/library/bb495757(v=vs.110).aspxAlso do not rely on the gc calling a dtor - it is not safe and can be called totally randomed, so use RC instead or expicit destroy()RC is not applicable. I'm doing unit tests for a non-GC container and trying to make sure all destructors are called properly. Example: unittest { auto a = List!int([S(0), S(1), S(2)]); a.popBack(); assert(equal(a[], [S(0), S(1)])); } // lots of similar unittests unittest { import std.stdio; GC.collect(); assert(S.count == 0); } So if all goes well, S.count should be zero, but the arrays I'm testing against are being allocated on the heap. Given the conditions of the tests, it seems like GC.collect should be able to reclaim those arrays after the unit tests have exited, and in most cases does. The ideal solution though, would be to allocate those arrays on the stack and avoid the problem altogether. There doesn't seem to be any reasonable way to do it though. // won't this allocate anyways? S[2] b = [S(0), S(1)]; assert(equal(a[], b[])); // why can't I just declare a static array inline? assert(equal(a[], int[2]{ S(0), S(1) }));
Aug 12 2017
On Saturday, 12 August 2017 at 17:25:36 UTC, bitwise wrote:On Saturday, 12 August 2017 at 08:16:56 UTC, Temtaime wrote:auto s(T, size_t n)(T[n] values) { return values; } assert(equal(a[], [S(0), S(1)].s));Collect - is a hint to the GC, not an order. It can ignore this request.If this is the case, then D's GC should have an option to force https://msdn.microsoft.com/en-us/library/bb495757(v=vs.110).aspxAlso do not rely on the gc calling a dtor - it is not safe and can be called totally randomed, so use RC instead or expicit destroy()RC is not applicable. I'm doing unit tests for a non-GC container and trying to make sure all destructors are called properly. Example: unittest { auto a = List!int([S(0), S(1), S(2)]); a.popBack(); assert(equal(a[], [S(0), S(1)])); } // lots of similar unittests unittest { import std.stdio; GC.collect(); assert(S.count == 0); } So if all goes well, S.count should be zero, but the arrays I'm testing against are being allocated on the heap. Given the conditions of the tests, it seems like GC.collect should be able to reclaim those arrays after the unit tests have exited, and in most cases does. The ideal solution though, would be to allocate those arrays on the stack and avoid the problem altogether. There doesn't seem to be any reasonable way to do it though. // won't this allocate anyways? S[2] b = [S(0), S(1)]; assert(equal(a[], b[])); // why can't I just declare a static array inline? assert(equal(a[], int[2]{ S(0), S(1) }));
Aug 12 2017
bitwise <bitwise.pvt gmail.com>
On Saturday, 12 August 2017 at 17:52:47 UTC, Dgame wrote:
[...]
auto s(T, size_t n)(T[n] values) {
return values;
}
assert(equal(a[], [S(0), S(1)].s));
This seems to work, but I'm trying to determine if it's 100%
guaranteed safe.
Tacking on nogc doesn't seem to stop it from working, so that
checks out, but my example print's "postblit" three times:
struct S {
int x;
this(int x) nogc { this.x = x; }
this(this) nogc { printf("postblit\n"); }
}
auto s(T, size_t n)(T[n] values) nogc {
return values;
}
void main(string[] args) nogc {
foreach(ref s; [S(0), S(1), S(2)].s)
printf("%d\n", s.x);
}
So I think what's happening is that the array is moved into the
argument of s(), then copied to the return value, which resides
on the parent stack frame of s(), meaning that it's safe from the
body of the foreach clobbering it...is that correct?
Assuming this is safe though, it would be nice to eliminate the
postblits too. It seems like it should be move-move instead of a
move-copy. I tried "return move(values);" but that still didn't
help.
Thanks
Aug 12 2017
On 8/12/17 4:16 AM, Temtaime wrote:Collect - is a hit to the GC, not an order. It can ignore this request.Hm.. where is this documented? I agree that the GC may not clean up all memory that could be legitimately cleaned up, but that's not because it didn't do a collection. -Steve
Aug 14 2017
On Saturday, 12 August 2017 at 08:16:56 UTC, Temtaime wrote:Collect - is a hit to the GC, not an order. It can ignore this request.As far as I can tell, it is an order. From the GC code[1]: void collect() nothrow { fullCollect(); } ... size_t fullCollect() nothrow { debug(PRINTF) printf("GC.fullCollect()\n"); // Since a finalizer could launch a new thread, we always need to lock // when collecting. static size_t go(Gcx* gcx) nothrow { return gcx.fullcollect(); } immutable result = runLocked!go(gcx); version (none) { GCStats stats; getStats(stats); debug(PRINTF) printf("heapSize = %zx, freeSize = %zx\n", stats.heapSize, stats.freeSize); } gcx.log_collect(); return result; } ... //Gcx.fullcollect size_t fullcollect(bool nostack = false) nothrow { MonoTime start, stop, begin; if (config.profile) { begin = start = currTime; } debug(COLLECT_PRINTF) printf("Gcx.fullcollect()\n"); //printf("\tpool address range = %p .. %p\n", minAddr, maxAddr); { // lock roots and ranges around suspending threads b/c they're not reentrant safe rangesLock.lock(); rootsLock.lock(); scope (exit) { rangesLock.unlock(); rootsLock.unlock(); } thread_suspendAll(); prepare(); if (config.profile) { stop = currTime; prepTime += (stop - start); start = stop; } markAll(nostack); thread_processGCMarks(&isMarked); thread_resumeAll(); } if (config.profile) { stop = currTime; markTime += (stop - start); Duration pause = stop - begin; if (pause > maxPauseTime) maxPauseTime = pause; start = stop; } ConservativeGC._inFinalizer = true; size_t freedLargePages=void; { scope (failure) ConservativeGC._inFinalizer = false; freedLargePages = sweep(); ConservativeGC._inFinalizer = false; } if (config.profile) { stop = currTime; sweepTime += (stop - start); start = stop; } immutable freedSmallPages = recover(); if (config.profile) { stop = currTime; recoverTime += (stop - start); ++numCollections; } updateCollectThresholds(); return freedLargePages + freedSmallPages; } 1. https://github.com/dlang/druntime/blob/master/src/gc/impl/conservative/gc.d
Aug 14 2017
Am Fri, 11 Aug 2017 18:44:56 +0000 schrieb bitwise <bitwise.pvt gmail.com>:[=E2=80=A6]That can't work and here is why: Druntime employs a conservative GC that will treat several things as potential pointers to GC memory. From the top of my head, the entire stack as well as void[] arrays and unions that contain pointers. Some integer variable on the stack or a chunk of a void[] that happens to have the same value as your GC pointer will keep it alive. Same for a union of an integer and a pointer where you have set the integer part to the address of your GC memory chunk. These misidentifications (false pointers) can become a real problem on 32-bit systems, where due to the small address space many things can look like valid pointers and keep GC memory alive that should long since have been recycled. P.S.: Also keep in mind that if you were to run multi-threaded, the ptr you test for could have been recycled and reassigned between GC.collect() and GC.addrOf(). Some unittesting frameworks for example run the tests in parallel. --=20 Marco
Aug 12 2017