Skip to content
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

Memory leak when using net.newContext() #19572

Open
tdely opened this issue Mar 1, 2022 · 1 comment
Open

Memory leak when using net.newContext() #19572

tdely opened this issue Mar 1, 2022 · 1 comment

Comments

@tdely
Copy link
Contributor

tdely commented Mar 1, 2022

SslContext from net.newContext() leaks memory when destroyed.

Example

import std/net

proc main() =
  for i in 1 .. 4:
    let ctx = newContext()

main()

Current Output

$ valgrind --leak-check=full ./test
==11241== Memcheck, a memory error detector
==11241== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==11241== Using Valgrind-3.18.1 and LibVEX; rerun with -h for copyright info
==11241== Command: ./test
==11241==
==11241==
==11241== HEAP SUMMARY:
==11241==     in use at exit: 2,343,489 bytes in 38,896 blocks
==11241==   total heap usage: 103,731 allocs, 64,835 frees, 8,222,641 bytes allocated
==11241==
==11241== 2,337,992 (4,096 direct, 2,333,896 indirect) bytes in 4 blocks are definitely lost in loss record 107 of 107
==11241==    at 0x48397B5: malloc (vg_replace_malloc.c:381)
==11241==    by 0x10AFBD: allocImpl__system_1725 (malloc.nim:5)
==11241==    by 0x115286: allocSharedImpl (malloc.nim:25)
==11241==    by 0x11A9C3: allocWrapper__wrappersZopenssl_578 (openssl.nim:551)
==11241==    by 0x4FB7FE9: CRYPTO_zalloc (in /usr/lib/x86_64-linux-gnu/libcrypto.so.1.1)
==11241==    by 0x5171FFE: SSL_CTX_new (in /usr/lib/x86_64-linux-gnu/libssl.so.1.1)
==11241==    by 0x11E5F2: newContext__pureZnet_1234 (net.nim:623)
==11241==    by 0x11FDCD: main__test_2 (test.nim:5)
==11241==    by 0x11FF26: NimMainModule (test.nim:7)
==11241==    by 0x11FF5A: NimMainInner (system.nim:2229)
==11241==    by 0x11FE84: NimMain (system.nim:2237)
==11241==    by 0x11FEA6: main (system.nim:2244)
==11241==
==11241== LEAK SUMMARY:
==11241==    definitely lost: 4,096 bytes in 4 blocks
==11241==    indirectly lost: 2,333,896 bytes in 38,880 blocks
==11241==      possibly lost: 0 bytes in 0 blocks
==11241==    still reachable: 5,497 bytes in 12 blocks
==11241==         suppressed: 0 bytes in 0 blocks
==11241== Reachable blocks (those to which a pointer was found) are not shown.
==11241== To see them, rerun with: --leak-check=full --show-leak-kinds=all
==11241==
==11241== For lists of detected and suppressed errors, rerun with: -s
==11241== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Additional Information

It's possible to work around this by using {.threadvar.} and not recreating the SslContext, like the default context is handled in the httpclient module: this should work in most cases. It won't work where the context needs to change, such as a configuration reload to use a new certificate and private key.

$ nim -v
Nim Compiler Version 1.6.4 [Linux: amd64]
Compiled at 2022-02-09
Copyright (c) 2006-2021 by Andreas Rumpf

git hash: 7994556f3804c217035c44b453a3feec64405758
active boot switches: -d:release
@Araq Araq added the Severe label Mar 9, 2022
@ghost
Copy link

ghost commented Oct 29, 2023

This is not really a memory leak, SslContext doesn't have a default destructor/finalizer, so you're supposed to call destroyContext manually:

import std/net

proc main() =
  for i in 1 .. 4:
    let ctx = newContext()
    ctx.destroyContext()

main()

Gives:

==205161== HEAP SUMMARY:
==205161==     in use at exit: 3,333 bytes in 9 blocks
==205161==   total heap usage: 177,623 allocs, 177,614 frees, 15,643,420 bytes allocated
==205161== 
==205161== LEAK SUMMARY:
==205161==    definitely lost: 0 bytes in 0 blocks
==205161==    indirectly lost: 0 bytes in 0 blocks
==205161==      possibly lost: 0 bytes in 0 blocks
==205161==    still reachable: 3,333 bytes in 9 blocks
==205161==         suppressed: 0 bytes in 0 blocks

So, this issue is either about:

  1. Adding a default destructor to SslContext
  2. Adding a reference to destroyContext in the docs to newContext

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

No branches or pull requests

3 participants