Hello,
I have a testcase cut down from a larger application that leaks memory with --gc:orc but does not leak memory with --gc:refc. The testcase uses the Decimal library from Nimble.
Are there steps that library providers need to take to support --gc:orc ? What should I investigate to determine if the issue is in the Decimal libarary or in the nim compiler ?
The testcase below leaks memory from the collect() macro when using nim v1.6.6 (git hash 0565a70eab02122ce278b98181c7d1170870865c) on Ubuntu 18.0.4:
import strutils, strformat
import decimal/decimal
import sugar
type
Timeval {.importc: "timeval", header:"<sys/time.h>", bycopy.} = object
Rusage* {.importc: "struct rusage", header:"<sys/resource.h>", bycopy.} = object
ru_utime {.importc.}: Timeval
ru_stime {.importc.}: Timeval
ru_maxrss* {.importc.}: int32 # Maximum resident set size
# ...
ru_minflt* {.importc.}: int32 # page reclaims (soft page faults)
RusageWho* {.size: sizeof(cint).} = enum
RusageChildren = -1
RusageSelf = 0
RusageThread = 1
proc getrusage*(who: RusageWho, usage: var Rusage) {.importc, header: "sys/resource.h".}
proc main() =
let constDcm = newDecimal("0.15")
var newRange = collect(for x in 0..<10000: constDcm + 1)
var ru: Rusage
getrusage(RusageSelf, ru)
let initRss = ru.ru_maxrss
var lastRss = initRss
let prInterval = 100
for i in 0..<1000:
# Operation which leaks memory with --gc:orc,
# but does not leak memoryy with --gc:refc
# Note - need the add operation with
# a Decimal in collect() below
newRange = collect(for x in 0..<10000: constDcm + 1)
# Attempt to reclaim memory
GC_fullCollect()
# Check memory consumption
getrusage(RusageSelf, ru)
if ru.ru_maxrss > 100 * initRss:
raise newException(ValueError,"\n\n!!!!! Leaking Memory !!!!!\n\n")
# Periodically print stats on memory consumption
if i mod prInterval == 0:
echo &"\nGC_getStatistics():\n{GC_getStatistics()}"
var incRss = ru.ru_maxrss - lastRss
lastRss = ru.ru_maxrss
echo &"Max RSS {ru.ru_maxrss}, Incremental max RSS: {incRss} for {prInterval} iterations"
echo()
when defined(nimTypeNames):
dumpNumberOfInstances()
echo()
echo "\n\n###### No Memory Leak ######\n\n"
when isMainModule:
main()
A much simpler way to check if memory leaks with orc is by using -d:useMalloc together with valgrind:
import std/sugar
import decimal/decimal
proc main() =
let constDcm = newDecimal("0.15")
var newRange = collect(for x in 0..<5: constDcm + 1)
# Operation which leaks memory with --gc:orc,
# but does not leak memoryy with --gc:refc
# Note - need the add operation with
# a Decimal in collect() below
newRange = collect(for x in 0..<5: constDcm + 1)
main()
And GC_fullCollect() isn't needed for when you have no cycles, so you can actually test with ARC:
nim c -d:danger -d:useMalloc --gc:arc ttaa.nim
==12848== LEAK SUMMARY:
==12848== definitely lost: 1,008 bytes in 21 blocks
==12848== indirectly lost: 336 bytes in 21 blocks
==12848== possibly lost: 0 bytes in 0 blocks
==12848== still reachable: 0 bytes in 0 blocks
==12848== suppressed: 0 bytes in 0 blocks
The leaks do seem to exist, but I think they actually come from the C code decimal uses, I'll try to investigate more.
I tried the addition of the system.`=destroy`(x[0])` as in the PR: https://github.com/nim-lang/Nim/issues/19794 and it still leaks as Araq indicated.
I'm now attempting to update the decimal package to use an object for DecimalType, but I don't yet have the Nim skills to get the constructor and destructor to compile.
How should I define the DecimalType object ?
Here is a snippet of the code upon which I have experimented and the associated Nim 1.6.6 compiler error:
when not compileOption("gc", "arc") and not compileOption("gc", "orc"):
type
DecimalType* = ref[ptr mpd_t]
proc deleteDecimal(x: DecimalType) =
if not x.isNil: # Managed by Nim
assert(not(x[].isNil)) # Managed by MpDecimal
mpd_del(x[])
proc newDecimal*(): DecimalType =
## Initialize a empty DecimalType
new result, deleteDecimal
result[] = mpd_qnew()
else:
# Attempt to enable ARC/ORC
type
DecimalType* = object
d : ptr mpd_t
proc deleteDecimal(x: DecimalType) =
if not x.d.isNil: # Managed by Nim
assert(not(x.d.isNil)) # Managed by MpDecimal
mpd_del(x.d)
proc newDecimal*(): DecimalType =
## Initialize a empty DecimalType
new result, deleteDecimal
result.d = mpd_qnew()
Compiler error:
Error: type mismatch: got <DecimalType, proc (x: DecimalType){.noSideEffect, gcsafe, locks: 0.}>
but expected one of:
proc new(t: typedesc): auto
first type mismatch at position: 1
required type for t: typedesc
but expression 'result' is of type: DecimalType
proc new[T](a: var ref T)
first type mismatch at position: 1
required type for a: var ref T
but expression 'result' is of type: DecimalType
proc new[T](a: var ref T; finalizer: proc (x: ref T) {.nimcall.})
first type mismatch at position: 1
required type for a: var ref T
but expression 'result' is of type: DecimalType
expression: new result, deleteDecimal
Try this variant please:
# Attempt to enable ARC/ORC
type
DecimalType* = object
d : ptr mpd_t
proc `=destroy`(x: var DecimalType) =
if not x.d.isNil: # Managed by Nim
assert(not(x.d.isNil)) # Managed by MpDecimal
mpd_del(x.d)
proc newDecimal*(): DecimalType =
## Initialize a empty DecimalType
result.d = DecimalType(d: mpd_qnew())
Thanks Araq.
With your suggestion and a definition of a [] for DecimalType to return the d attribute of the object, I'm able to compile and partially run. The execution progresses somewhat and then fails an assertion down in the mpd C code.
One more question. Using one of my stress-tests and the =destroy proc defined as below, I get free(): double free detected in tcache 2 and a core dump in =destroy at the call to mpd_del. Without the =destroy the stress-test succeeds, but it obviously leaks memory.
I'm missing something on how to construct the =destroy proc. Any advice would be very helpful.
The declaration of the mpd_del proc is below:
proc mpd_del*(dec: ptr mpd_t) {.cdecl, importc: "mpd_del", header: cHeader.}
The DeviceType creation and destruction procs are below:
type
DecimalType* = object
d : ptr mpd_t
proc `[]`*(x: DecimalType): ptr mpd_t =
result = x.d
proc `=destroy`(x: var DecimalType) =
# if not x.d.isNil: # Managed by Nim
if x.d != nil:
mpd_del(x.d)
proc newDecimal*(): DecimalType =
## Initialize a empty DecimalType
result = DecimalType(d: mpd_qnew())
You're not setting x.d to nil, so two calls to the destructor will result in a double free.
Nim shouldn't be calling the destructor twice so do post a mwe if you can but I'd try:
proc `=destroy`(x: var Decimal)=
if not x.d.isNil:
mpd_del(x.d)
x.d = nil
Thanks,
Moving =copy allowed compilation and execution.
Now =copy gets called with destination d attribute equal to nil. I'm checking to see if there is an issue with newDecimal in case it creates "empty" DecimalTypes.
Thanks again
I cut down a testcase which exhibits the core dump due to =copy getting called with dest.d.
import decimal/decimal
import sugar
proc main() =
let constDcm = newDecimal("0.15")
var newRange = collect(for x in 0..<2: constDcm + 1)
var varDcm = constDcm
varDcm = newDecimal("0.10")
newRange = collect(for x in 0..<2: constDcm + 1)
when isMainModule:
main()
The call to =copy with destination nil occurs at the var varDcm = constDcm statement. Interestingly, deleting any of the statements following the offending statement avoids the issue.
Any guidance on how to debug further ?
Thanks for all of the help so far.
Yes. That line was incorrect.
Here are the DecimalType creation and destruction routines as I have them now:
when compileOption("gc", "arc") or compileOption("gc", "orc"):
type
DecimalType* = object
d : ptr mpd_t
proc `[]`*(x: DecimalType): ptr mpd_t =
result = x.d
proc `=destroy`(x: var DecimalType) =
echo &"destroying: {x}"
if not x.d.isNil: # Managed by Nim
mpd_del(x.d)
x.d = nil
proc `=copy`*(dest: var DecimalType, source: DecimalType) =
echo &"dest: {dest}"
if dest.d == nil : echo "dest.d is nil"
echo &"source: {source}"
if source.d == nil : echo "source.d is nil"
if dest.d != source.d:
`=destroy`(dest)
wasMoved(dest)
mpd_copy(dest.d, source.d, CTX_ADDR)
proc newDecimal*(): DecimalType =
## Initialize a empty DecimalType
result = DecimalType(d: mpd_qnew())
if result.d == nil :
echo "##### Empty new Decimal #####"
else:
echo "good new Decimal"
Here is the testcase along with its output.
import decimal/decimal
import sugar
proc main() =
echo "let constDcm"
let constDcm = newDecimal("0.15")
echo "newRange"
var newRange = collect(for x in 0..<2: constDcm + 1)
echo "var varDcm"
var varDcm = constDcm
echo "varDcm"
varDcm = newDecimal("0.10")
echo "newRange"
newRange = collect(for x in 0..<2: constDcm + 1)
when isMainModule:
main()
Output with the core dump immediately following the last output line.
let constDcm
good new Decimal
newRange
good new Decimal
good new Decimal
destroying: (d: ...)
good new Decimal
good new Decimal
destroying: (d: ...)
var varDcm
dest: (d: ...)
dest.d is nil
source: (d: ...)
destroying: (d: ...)
Nice I see what's the issue, there is a version where mpd_t struct is dynamically allocated and another where it's static https://www.bytereef.org/mpdecimal/doc/libmpdec/memory.html#advanced-memory-handling
So before calling mpd_copy you need to call mpd_new on dest inside =copy. Unfortunately Nim doesn't have implicitly called constructors.
Great insight! Thanks !
I added a dest.d = mpd_new(CTX_ADDR) immediately prior to the call to mpd_copy and the testcase executes without error. Additionally, the stress test that had been failing is running without error so far.
Once I'm more skilled, I will need to wrap a more-performant, fixed-precision, bid-encoded decimal library such as Intel's decimal library for my application.
Thanks to everyone for the support !