Closed dunhamsteve closed 2 years ago
Looking at support/c/getline.c
, it seems this code isn't ours; it was copied from a 2011 version of a file included with NetBSD. I wonder if the issue could be fixed by swapping in a more recent version of the same library (for example, OpenBSD's getdelim.c and getline.c)?
The man page says that the libc version of getline
mallocs a buffer if null, and expects the caller to free it. Looks like openbsd requires a buffer to be passed in and gives an error otherwise. It looks like the Idris support code expects the caller to free it:
Chez (9.5.7 on mac) does not seem to be freeing it though. I tested Linux with the latest (9.5.9) above. Maybe older versions of Chez free void*
pointers, but I didn't see anything in the Chez release notes. I will retest on Linux with the 9.5 chez that ubuntu distributes.
It's reproducing for me with the 9.5 chez in ubuntu 20.04.3
This quick and dirty patch fixes it, but will probably break other backends. It is just to test if freeing the pointer works.
We should check if it is also an issue on racket. That pointer has to be passed back from C to scheme with a string
declaration before the underlying buffer is freed. If we could get chez to manage the void*
that would be nice. Otherwise, we might want to change prim__readLine
to return string
and take care of business in the scheme side of the support function.
diff --git a/libs/base/System/File/ReadWrite.idr b/libs/base/System/File/ReadWrite.idr
index 4d7030e0..f741d899 100644
--- a/libs/base/System/File/ReadWrite.idr
+++ b/libs/base/System/File/ReadWrite.idr
@@ -8,6 +8,7 @@ import System.File.Handle
import public System.File.Error
import System.File.Support
import public System.File.Types
+import System.FFI
%default total
@@ -61,7 +62,10 @@ fGetLine (FHandle f)
= do res <- primIO (prim__readLine f)
if prim__nullPtr res /= 0
then returnError
- else ok (prim__getString res)
+ else do
+ let s = prim__getString res
+ free $ prim__forgetPtr res
+ ok s
||| Get a number of characters from the given file handle.
|||
It feels a bit weird to have those primitives implemented in C instead of calling out to the Scheme backend. I guess it would have to be fixed for the RefC backend either way, so it wouldn't be easier to move it to Scheme, I don't remember, was performance the reason or was there some other consideration?
Racket version 8.4 [cs] has the same issue. I suspect every scheme has the issue since they can't know the difference between prim__getEnv
(should never be freed) and prim__readLine
(should always be freed), both of which return a PrimIO (Ptr String)
. I think it is Ptr String
rather than String
to allow nulls.
Process 17101: 168717 leaks for 172776368 total leaked bytes.
STACK OF 138089 INSTANCES OF 'ROOT LEAK: <malloc>':
7 dyld 0x231e3fc10 start + 2368
6 idris2-boot 0x100aeabec main + 1712
5 idris2-boot 0x100aeb2d0 racket_boot + 1512
4 idris2-boot 0x100b7b004 Scall2 + 120
3 ??? 0x10acc0900 0x7fffffffffffffff + 9223372041330886913
2 libidris2_support.dylib 0x105c664f4 idris2_readLine + 32
1 libidris2_support.dylib 0x105c660b4 getdelim + 72
0 libsystem_malloc.dylib 0x1a58ef42c _malloc_zone_malloc + 272
Idris appears to be leaking memory in FFI on scheme. This was first noticed because the LSP process was growing as I was working on Idris. Repeatedly reloading a touched
Desugar.idr
caused the process to grow in size. Further investigation revealed that both theleaks
app on macos andvalgrind
on linux report that memory is leaked by thedelim
function iniogetdelim.c
. I believe the malloc'd string needs to be free'dSteps to Reproduce
In the Idris2 source directory:
On Linux run:
On MacOS:
and while it is running, run
leaks
with the PID.Expected Behavior
Expect no leaks to be reported:
Observed Behavior
MacOS reports:
Linux reports: