Skip to content

Create or document way to clear all global memory #21403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
PMunch opened this issue Feb 20, 2023 · 8 comments
Open

Create or document way to clear all global memory #21403

PMunch opened this issue Feb 20, 2023 · 8 comments

Comments

@PMunch
Copy link
Contributor

PMunch commented Feb 20, 2023

Summary

This came about when working with dynamic libraries. If you write a dynamic library in Nim and then load it into a separate program there is no (documented) way to free all the memory if the loader program unloads the dynamic library. This means that if a system loads and unloads a dynamic library it will keep increasing in memory.

Description

What I propose is to create a procedure NimQuit opposite of NimMain which will destroy all memory. Calling any other code using GC after that will be undefined behaviour. This would then be called automatically in .fini by attaching __attribute((destructor))__ or hooking in DllMain, similar to how NimMain is called today. And analogous to how --noMain disables this behaviour --noQuit should prevent it from being called and NimQuit must be called exlicitly.

Alternatives

No response

Examples

As a simple example/testing ground I wrote this:

loader.nim:

import std/dynlib
type
  InitFunction = proc () {.cdecl.}
  DeinitFunction = proc () {.cdecl.}

proc loadDynamic() =
  let lib = loadLib("./liblibrary.so")
  assert lib != nil, "Error loading library"

  let init = cast[InitFunction](lib.symAddr("dynlib_init"))
  assert init != nil, "Error loading 'init' function from library"
  let deinit = cast[DeinitFunction](lib.symAddr("dynlib_deinit"))
  assert deinit != nil, "Error loading 'deinit' function from library"

  init()
  deinit()

  unloadLib(lib)

while true:
  loadDynamic()

library.nim:

var x = newSeq[byte](uint16.high)

proc NimMain() {.cdecl, importc.}

proc dynlib_init() {.cdecl, exportc, dynlib.} =
  NimMain()
  echo "Loaded dynamic library"

proc dynlib_deinit() {.cdecl, exportc, dynlib.} =
  GC_FullCollect()

Compiled with:

nim c --gc:orc loader.nim

and

nim c --app:lib --noMain --gc:orc library.nim

Backwards Compatibility

No response

Links

No response

@PMunch
Copy link
Contributor Author

PMunch commented Feb 20, 2023

After writing this I discovered deallocHeap in the module gc_common, unfortunately this isn't available for ARC/ORC it seems.

@PMunch
Copy link
Contributor Author

PMunch commented Feb 20, 2023

Did some testing and exposing deallocOsPages and calling that in the dynlib_deinit indeed made the memory consumption stop increasing. This won't call finalizers/destructors though, which isn't great, but at least it points to deallocHeap as the thing that has to be implemented for ARC/ORC in order to make this possible.

@ringabout
Copy link
Member

related #18262

@PMunch
Copy link
Contributor Author

PMunch commented Mar 1, 2023

Just for reference if someone wants to give a go at implementing this. I believe that this procedure is what needs to be ported to ARC/ORC and then create a NimExit which basically just calls it:

proc deallocHeap*(runFinalizers = true; allowGcAfterwards = true) =
## Frees the thread local heap. Runs every finalizer if `runFinalizers`
## is true. If `allowGcAfterwards` is true, a minimal amount of allocation
## happens to ensure the GC can continue to work after the call
## to `deallocHeap`.
template deallocCell(x) =
if isCell(x):
# cast to PCell is correct here:
prepareDealloc(cast[PCell](x))
if runFinalizers:
when not declared(allObjectsAsProc):
for x in allObjects(gch.region):
deallocCell(x)
else:
var spaceIter: ObjectSpaceIter
while true:
let x = allObjectsAsProc(gch.region, addr spaceIter)
if spaceIter.state < 0: break
deallocCell(x)
deallocOsPages(gch.region)
zeroMem(addr gch.region, sizeof(gch.region))
if allowGcAfterwards:
initGC()

@Araq
Copy link
Member

Araq commented Jun 7, 2023

Makes sense but this new NimExit won't run destructors, it simply cannot as the RTTI is not always available.

@PMunch
Copy link
Contributor Author

PMunch commented Jun 7, 2023

Hmm, that's a bit unfortunate. But still better than just leaving all the memory dangling. Does deallocHeap imply GC_FullCollect? Let's say I have an object which wraps a file and the destructor flushes my changes. If my program ends while that object still exists the changes won't be flushed (poor design, but it's just an example). Since deallocHeap can't run this destructor (because it can't see which type it is) I would then have to ensure that it is freed before I exit. If I then set my reference to this object to nil and call GC_FullCollect this destructor should be run, but if I omit the GC_FullCollect call does deallocHeap then call this destructor? My guess is no.

@Araq
Copy link
Member

Araq commented Jun 7, 2023

deallocHeap should not call GC_FullCollect but NimExit should call it.

@commandline-be
Copy link

deallocHeap should not call GC_FullCollect but NimExit should call it.

is this also addressed by e217bb2 ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants