chiark / gitweb /
basic/hashmap: add cleanup of memory pools (#7164)
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Fri, 10 Nov 2017 14:44:58 +0000 (15:44 +0100)
committerSven Eden <yamakuzure@gmx.net>
Fri, 10 Nov 2017 14:44:58 +0000 (15:44 +0100)
commitcd30037fde29c36430cce29c8c82c70bb7c7abd2
tree7d8489ac3b561f6b38b06a50a67d3d4d385e101c
parentf56ba2f20e14bd1422ea3b080772b602d079a8fc
basic/hashmap: add cleanup of memory pools (#7164)

It was dropped in 89439d4fc0d29f04ac68432fd06ab84bc4e36e20. As a result, every
process that uses a hashmap allocates and then leaks the hashmap mempools.
The mempools are only allocated in the main thread, but we don't know where
the memory is used.

So let's check if we are the last thread and free the mempools then. This is
fairly heavy, because /proc/self/status has to be opened and parsed, but we do
it only when compiled for valgrind, i.e. not by default, and compared to running
under valgrind or asan, the extra cost is acceptable. The big advantage is that
we don't have to think or filter out this false positive.

As a micro-opt, cleanup is attempted only in the main thread. We could allow
any thread to check if it is the last one and perform cleanup, but that'd mean
that we'd have to _do_ the check in every thread. We don't use threads like
that, our non-main threads are always short-lived, so let's just accept the
possibility that we'll leak memory if a thread survives. The check is also
non-atomic, but it's called in a destructor of the main thread _and_ we do
cleanup only when there are no other threads, so the risk of some library
suddenly spawning another thread is very low. All in all, this is not perfect,
but should work in 999‰ of cases.

Fixes the following valgrind warning:

==22564== HEAP SUMMARY:
==22564==     in use at exit: 8,192 bytes in 2 blocks
==22564==   total heap usage: 243 allocs, 241 frees, 151,905 bytes allocated
==22564==
==22564== 4,096 bytes in 1 blocks are still reachable in loss record 1 of 2
==22564==    at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==22564==    by 0x4F08A8C: mempool_alloc_tile (mempool.c:62)
==22564==    by 0x4F08B16: mempool_alloc0_tile (mempool.c:81)
==22564==    by 0x4EF8DE0: hashmap_base_new (hashmap.c:748)
==22564==    by 0x4EF8ED9: internal_hashmap_new (hashmap.c:782)
==22564==    by 0x11045D: test_hashmap_copy (test-hashmap-plain.c:87)
==22564==    by 0x115722: test_hashmap_funcs (test-hashmap-plain.c:914)
==22564==    by 0x10FC9D: main (test-hashmap.c:60)
==22564==
==22564== 4,096 bytes in 1 blocks are still reachable in loss record 2 of 2
==22564==    at 0x4C2FB6B: malloc (vg_replace_malloc.c:299)
==22564==    by 0x4F08A8C: mempool_alloc_tile (mempool.c:62)
==22564==    by 0x4F08B16: mempool_alloc0_tile (mempool.c:81)
==22564==    by 0x4EF8DE0: hashmap_base_new (hashmap.c:748)
==22564==    by 0x4EF8EF8: internal_ordered_hashmap_new (hashmap.c:786)
==22564==    by 0x10A2A0: test_ordered_hashmap_copy (test-hashmap-ordered.c:89)
==22564==    by 0x10F70F: test_ordered_hashmap_funcs (test-hashmap-ordered.c:916)
==22564==    by 0x10FCA2: main (test-hashmap.c:61)
==22564==
==22564== LEAK SUMMARY:
==22564==    definitely lost: 0 bytes in 0 blocks
==22564==    indirectly lost: 0 bytes in 0 blocks
==22564==      possibly lost: 0 bytes in 0 blocks
==22564==    still reachable: 8,192 bytes in 2 blocks
==22564==         suppressed: 0 bytes in 0 blocks

v2:
- check if we are the main thread

v3:
- check if there are no other threads
src/basic/hashmap.c