[Issue 4358] Potential Memory Leaks in std.file.read() ?
https://issues.dlang.org/show_bug.cgi?id=4358 Mathias LANG changed: What|Removed |Added Status|NEW |RESOLVED CC||pro.mathias.l...@gmail.com Resolution|--- |WORKSFORME --- Comment #9 from Mathias LANG --- Since it's a GC problem (hence Druntime, not DMD) and well known, I don't think there's something actionable in this bug report. Additionally, the guidelines provided in this bug report (don't allocate very large arrays with the GC), switching to 64 bits, and the improvements that have been made to the GC should greatly mitigate the problem. --
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 --- Comment #8 from Leandro Lucarella 2010-06-22 10:03:22 PDT --- (In reply to comment #7) > std.file.read returns a void[], but it's allocated manually via gc.malloc(), > and I think the no-pointers thing was done right. And what is the rationale for not returning ubyte[]? > The correct solution to this problem would be to apply the patch in bug 3463 > (precise heap scanning), and then probably investigate precise stack scanning. You can't have full precise scanning in D, because of unions and other low-level constructs, so the problem of false pointers will be still there even with that patch (of course the chances of having a false pointer will be much lower). > I believe the patch in bug 2927 (no interior pointers attribute) would not > roll > well with Andrei's safety ideology, so let's forget that. That's stupid, it's not unsafer than NO_SCAN or casting a pointer to int, or whatever construct you like. That's obviously a construct that not everybody will use. So let's not forget that (and putting words in somebody else's mouth is not a good argument either =). > Note that enhancement 2927 could be simulated by using a wrapper object, that > behaves like an array, but actually uses C malloc to allocate memory. When the > wrapper gets collected, its finalizer can free() the memory. Or using > reference > counting, like (I believe) Andrei's TightArray (or what was it named) does. That's true, is a little more convoluted than having NO_INTERIOR and is not as easy to use by the compiler itself, but could be another option. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 --- Comment #7 from nfx...@gmail.com 2010-06-22 08:23:51 PDT --- std.file.read returns a void[], but it's allocated manually via gc.malloc(), and I think the no-pointers thing was done right. The correct solution to this problem would be to apply the patch in bug 3463 (precise heap scanning), and then probably investigate precise stack scanning. I believe the patch in bug 2927 (no interior pointers attribute) would not roll well with Andrei's safety ideology, so let's forget that. Note that enhancement 2927 could be simulated by using a wrapper object, that behaves like an array, but actually uses C malloc to allocate memory. When the wrapper gets collected, its finalizer can free() the memory. Or using reference counting, like (I believe) Andrei's TightArray (or what was it named) does. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 --- Comment #6 from Leandro Lucarella 2010-06-22 08:11:50 PDT --- (In reply to comment #5) > (In reply to comment #3) > > Most likely, the GC _is_ working correctly. It's a conservative GC, and it > > treats integers, floats, random binary data the same as actual pointers. > > This is false. The GC only treats void[] as a potential source of pointers, > which makes sense, really. int[], float[], char[], byte[] are *NOT* scanned > for > pointers. For a binary buffer that doesn't have pointers in it, you should > probably use ubyte[]. If std.file.read() return void[], I'd say that > std.file.read() is broken. I should add that, even when ubyte[] is not scanned for pointers, you are right about allocating big chunks of memory in the GC could lead to leaks, as the probabilities of having a false pointer pointing to that chunk of data gets really high. There is another bug report (with a patch too, and from David Simcha too) that helps with this problem: bug 2927. The idea is to add a new property to the GC to mark a chunk as not having interior pointers. If you mark a chunk of memory as not having interior pointers, the chances of a false pointer pointing to the beginning of your memory chunk becomes *very* low. It's a real shame that David's patches didn't get into druntime, as they are very helpful avoiding this kind of issues. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 Leandro Lucarella changed: What|Removed |Added CC||llu...@gmail.com --- Comment #5 from Leandro Lucarella 2010-06-22 08:05:07 PDT --- (In reply to comment #3) > Most likely, the GC _is_ working correctly. It's a conservative GC, and it > treats integers, floats, random binary data the same as actual pointers. This is false. The GC only treats void[] as a potential source of pointers, which makes sense, really. int[], float[], char[], byte[] are *NOT* scanned for pointers. For a binary buffer that doesn't have pointers in it, you should probably use ubyte[]. If std.file.read() return void[], I'd say that std.file.read() is broken. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 --- Comment #4 from Vermi 2010-06-22 00:23:46 PDT --- (In reply to comment #3) > Most likely, the GC _is_ working correctly. It's a conservative GC, and it > treats integers, floats, random binary data the same as actual pointers. The > GC > doesn't have enough information to know what is pointer and what not. If an > integer value looks like a pointer to a memory block, it's called a "false > pointer". This may lead to memory leaks. > > And the larger a memory block is, the higher the probability that a false > pointer exists and will prevent the memory block from being free'd. At least > that's my theory. Conclusion: use malloc() for really large arrays. Maybe, depend of how the GC scan the stack. I will think about it. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 --- Comment #3 from nfx...@gmail.com 2010-06-22 00:15:35 PDT --- Most likely, the GC _is_ working correctly. It's a conservative GC, and it treats integers, floats, random binary data the same as actual pointers. The GC doesn't have enough information to know what is pointer and what not. If an integer value looks like a pointer to a memory block, it's called a "false pointer". This may lead to memory leaks. And the larger a memory block is, the higher the probability that a false pointer exists and will prevent the memory block from being free'd. At least that's my theory. Conclusion: use malloc() for really large arrays. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 --- Comment #2 from Vermi 2010-06-21 23:55:49 PDT --- It's very strange, I never had this kind of problem before. I must first finish my software, so I will add delete() in the code, but as soon as I have free time I will look if I can help with the D :) (I will take a look at GC's implementation). To answer you yes, imareges.dll is about 20 MB. I tried with a ~2MB file, and all worked well, so I guess there a limit size where the GC stop working correctly. I'll make some try this winter to see if I can make something. I take a look in the patch, but I need more than 2 minutes to understand the whole way the memory is managed in D. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
[Issue 4358] Potential Memory Leaks in std.file.read() ?
http://d.puremagic.com/issues/show_bug.cgi?id=4358 nfx...@gmail.com changed: What|Removed |Added CC||nfx...@gmail.com --- Comment #1 from nfx...@gmail.com 2010-06-21 21:55:39 PDT --- Congratulations, you just found out that D's GC implementation is fucking shit. There doesn't seem to be a bug in std.file.read. It just uses the GC to allocate a big chunk of memory, and returns it. There are probably false pointers. Note that the latest allocated buffer will not be free'd with fullCollect() (because you still have a reference to the buffer in that variable), but I assume imageres.dll is much smaller than 200 MB. Rule of thumb: never allocate big arrays with the GC. There's a patch in bug 3463 to alleviate the problem, but it seems to be abandoned. Though, there is one potential problem in the D2 version of std.file.read. It contains this line: auto buf = GC.malloc(size, GC.BlkAttr.NO_SCAN)[0 .. size]; buf is returned as void[] to the user. As far as I understand the recently introduced changes to array appending, the memory block for an array slice contains a hidden length field. This is not present in this case, thus random things may happen if you try to append to the returned array. I do not know if this actually is a problem. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email --- You are receiving this mail because: ---
