Closure iterators can leak resources and there is no easy way to prevent it

[pmetras]

A closure iterator can leak memory or other critical resource and there is no easy way to prevent it. In the following example, the resource is a file, but it could be a system lock or a database cursor or shared memory.

Example

import os

proc bar(name: string): iterator: string =
  iterator foo: string {.closure.} =
    echo "Open file ", name
    var file = open(name)
    var line: string
    while readLine(file, line):
      yield line
    echo "Close file ", name
    close(file)

  result = foo

echo "Program starts"
var iter = bar(paramStr(1))

var i = 0
for l in iter():
  inc(i)
  echo "Read line #", i, "=", l
  if i > 5:
    break;

echo "Program completed"

Current Output

Compile with nim c -d:useMalloc --gc:arc c.nim.

Calling it with a text file containing more than 5 lines with valgrind using its source code as parameter:

$ valgrind --leak-check=full --show-leak-kinds=all ./c c.nim
==24524== Memcheck, a memory error detector
==24524== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==24524== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==24524== Command: ./c c.nim
==24524== 
Program starts
Open file c.nim
Read line #1=import os
Read line #2=
Read line #3=proc bar(name: string): iterator: string =
Read line #4=  iterator foo: string {.closure.} =
Read line #5=    echo "Open file ", name
Read line #6=    var file = open(name)
Program completed
==24524== 
==24524== HEAP SUMMARY:
==24524==     in use at exit: 552 bytes in 1 blocks
==24524==   total heap usage: 41 allocs, 40 frees, 13,896 bytes allocated
==24524== 
==24524== 552 bytes in 1 blocks are still reachable in loss record 1 of 1
==24524==    at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==24524==    by 0x4EBAE49: __fopen_internal (iofopen.c:65)
==24524==    by 0x4EBAE49: fopen@@GLIBC_2.2.5 (iofopen.c:89)
==24524==    by 0x10A060: open__gq12VLhVO0NBzUTnGgz4nw (in /home/pierre/c)
==24524==    by 0x10A7B5: open__cb1k9citqyT9a9brBSJAz8pkg (in /home/pierre/c)
==24524==    by 0x11229B: foo__KNRW21oWsgrenJ7T8WsgHw (in /home/pierre/c)
==24524==    by 0x112B04: NimMainModule (in /home/pierre/c)
==24524==    by 0x11286A: NimMainInner (in /home/pierre/c)
==24524==    by 0x1128A6: NimMain (in /home/pierre/c)
==24524==    by 0x1128F4: main (in /home/pierre/c)
==24524== 
==24524== LEAK SUMMARY:
==24524==    definitely lost: 0 bytes in 0 blocks
==24524==    indirectly lost: 0 bytes in 0 blocks
==24524==      possibly lost: 0 bytes in 0 blocks
==24524==    still reachable: 552 bytes in 1 blocks
==24524==         suppressed: 0 bytes in 0 blocks
==24524== 
==24524== For counts of detected and suppressed errors, rerun with: -v
==24524== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

There is no way to close the file used by the iterator when it is aborted.

Expected Output

When called with a file with less than 5 lines, the iterator completes and the file resource is released.

$ echo "It works" > f
$ valgrind --leak-check=full --show-leak-kinds=all ./c f
==24573== Memcheck, a memory error detector
==24573== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==24573== Using Valgrind-3.13.0 and LibVEX; rerun with -h for copyright info
==24573== Command: ./c f
==24573== 
Program starts
Open file f
Read line #1=It works
Close file f
Program completed
==24573== 
==24573== HEAP SUMMARY:
==24573==     in use at exit: 0 bytes in 0 blocks
==24573==   total heap usage: 23 allocs, 23 frees, 13,072 bytes allocated
==24573== 
==24573== All heap blocks were freed -- no leaks are possible
==24573== 
==24573== For counts of detected and suppressed errors, rerun with: -v
==24573== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

Possible Solution

• No easy solution because the Nim language does not have a way to give back control when a closure is destroyed (and particularly aborted).
• One could define a sentinel object in the closure iterator and define this object type destructor =destroy, managing the iterator status (completed or aborted) and a reference to the resource, to be called back when the iterator variable is destroyed, and then release the resource.
• Or register a close procedure with addQuitProc, but this limits the number such iterators used in the program...
• Or change the API to let the client manage the resource, outside of the iterator. This can make the API much more complex for the client code, or one wants to hide the implementation of an algorithm that could change.

Additional Information

• Example execution was shown with --gc:arc but the same problem exists with the default GC.
• try: ... finally: ... is of no use as there is no exception involved.
• Python has context managers and for ... else: ... to deal with this type of problem. I think the best way to deal with this problem would be to have a new language construct to deal with context/resource management, as resource management would be local (lease/release in the same part of the source code), giving less opportunity for bugs.

$ nim --version
Nim Compiler Version 1.1.1 [Linux: amd64]
Compiled at 2020-01-29
Copyright (c) 2006-2019 by Andreas Rumpf

active boot switches: -d:release

[alaviss]

I'd recommend that you put your code in a main() proc, since global variables are not destroyed (this still doesn't make the iterator finish, though).

[pmetras]

Yes. This example is not about memory leaks from variables but leaks from resources. Valgrind is used only to make visible that the file can't be closed when the iterator does not finish, with memory leaking as a side effect. With variables global or local to a main procedure, the result is the same.

[pmetras]

Here is the way to solve this type of problem as hinted here and demonstrated by @Yardanico in #15394.

One has to encapsulate the resource in an object type and define a destructor for that type.

import os

type
  AutoClose = object
    file: File

proc `=destroy`(ac: var AutoClose) =
  echo "Destroying AutoClose"
  ac.file.close()
  echo "File closed"

proc bar(name: string): iterator: string =
  iterator foo: string {.closure.} =
    echo "Open file ", name
    var ac = AutoClose(file: open(name))
    var line: string
    while readLine(ac.file, line):
      yield line
    echo "Close file ", name
    close(ac.file)

  result = foo

echo "Program starts"
var iter = bar(paramStr(1))

var i = 0
for l in iter():
  inc(i)
  echo "Read line #", i, "=", l
  if i > 5:
    break;

echo "Program completed"

When executed, on can see that the file resource is released and correctly closed.

 ./bug bug.nim 
Program starts
Open file bug.nim
Destroying AutoClose
File closed
Read line #1=import os
Read line #2=
Read line #3=type
Read line #4=  AutoClose = object
Read line #5=    file: File
Read line #6=
Program completed
Destroying AutoClose
File closed

Running valgrind shows that everything is fine now

$ valgrind ./bug bug.nim 
==9683== Memcheck, a memory error detector
==9683== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==9683== Using Valgrind-3.15.0 and LibVEX; rerun with -h for copyright info
==9683== Command: ./bug bug.nim
==9683== 
Program starts
Open file bug.nim
Destroying AutoClose
File closed
Read line #1=import os
Read line #2=
Read line #3=type
Read line #4=  AutoClose = object
Read line #5=    file: File
Read line #6=
Program completed
Destroying AutoClose
File closed
==9683== 
==9683== HEAP SUMMARY:
==9683==     in use at exit: 0 bytes in 0 blocks
==9683==   total heap usage: 3 allocs, 3 frees, 5,592 bytes allocated
==9683== 
==9683== All heap blocks were freed -- no leaks are possible
==9683== 
==9683== For lists of detected and suppressed errors, rerun with: -s
==9683== ERROR SUMMARY: 0 errors from 0 contexts (suppressed: 0 from 0)

[jlokier]

Seems to me this ought to work with defer: close in the closure, running all defer blocks when the closure is destroyed because that's when the lexical scope is destroyed. That doesn't work though, and seems like a language issue to be honest, because defer and AutoClose-style objects are in many respects two different ways of expressing the same thing.

[pmetras]

@jlokier, this forum thread explains the real behaviour of defer:.