nim-lang / Nim

Nim is a statically typed compiled systems programming language. It combines successful concepts from mature languages like Python, Ada and Modula. Its design focuses on efficiency, expressiveness, and elegance (in that order of priority).
https://nim-lang.org
Other
16.27k stars 1.46k forks source link

-d:useMalloc for the default GC #15394

Open mratsim opened 3 years ago

mratsim commented 3 years ago

At Status, we have been fighting against resource leaks for months (sockets, file descriptors, futures, timers and most importantly memory)

We built several tools to help us track the issues

However we are still leaking. As a workaround we are currently advising people to restart every 6 hours or so but for production we need to remove all possible source of leaks so that user can run that application for months without restart.

The computational part of our application is relatively easy to debug for leaks but the async/IO/networking part as been leaking resources quite often via closure iterators/futures due to unattended cancellation/expiration.

This is incredibly hard to debug.

We would like to have -d:useMalloc available for the default GC, backported to the 1.2.x branch so that we can use conventional C tools like Valgrind to detect those memory leaks and also run memory leak detection in dedicated CI.

Furthermore, we are currently investigating an issue with misreported memory accounting by Nim GC, which makes it even harder to debug with Nim standard tools (memory fragmentation?).

Araq commented 3 years ago

That seems to be more work than moving to 1.4's --gc:orc.

pmetras commented 3 years ago

Even with 1.4 and --gc:orc, there are situations where resources can leak: https://github.com/nim-lang/Nim/issues/13289. From what I understand, --gc:orc only tries to prevent memory leaks but does not help to deal with other types of leaks.

ghost commented 3 years ago

@pmetras wdym? with ORC no memory leaks should happen, otherwise it's a bug in ARC/ORC itself or in code which used manual memory allocation or with custom destructors (if they're not written correctly)

pmetras commented 3 years ago

@Yardanico , I agree that ORC solves memory leaks. I just want to say that there are other types of leaks, like listed by mratsim (sockets, file descriptors, futures, timers, BD cursors, OS locks, etc.) and that ORC does not help with these types of leaks.

ghost commented 3 years ago

@pmetras but it can and that's one of the reasons why ARC/ORC was created in the first place. You can create a custom destructor for any types including sockets, file descriptors, futures, locks, etc

ghost commented 3 years ago

@pmetras just for a simple example (compile with arc/orc for reliable results, or add GC_fullCollect() after main with refc)

type
  MyFile = object
    f: File

proc `=destroy`(myf: var MyFile) = 
  echo "destroying"
  myf.f.close() # close already does the isNil check for us

proc main = 
  let myf = MyFile(f: open("a.nim"))
  echo myf.f.readAll()

main()

I use a wrapper object MyFile here because you can't create destructors for pointers (and File is a "ptr CFile"). So as you can see it's not even hard

pmetras commented 3 years ago

@Yardanico, OK I understand how it can work: I'll have to encapsulate resource handles into custom types with a finalizer. I had an example working too but did not post it. Perhaps the File type in stdlib will be changed to become autoclosing in 1.6? Thanks for your example.

arnetheduck commented 3 years ago

That seems to be more work than moving to 1.4's --gc:orc.

The litmus test here is that not even the compiler works with orc/arc by default - Nimbus is quite a bit a larger project that is expected to run over long periods of time without restard, and that that has been tuned and optimized to the current GC - introducing a new GC, a new memory layout for core types like string and seq etc is a major risk given that it will bring new bugs, require new workarounds and require reoptimizing the memory profile from scratch - it's not something we can do frivolously. We're pretty much aware of how orc / arc works but it's not what we need in this particular case.