It was dropped in 89439d4fc0. 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
The advantage is that is the name is mispellt, cpp will warn us.
$ git grep -Ee "conf.set\('(HAVE|ENABLE)_" -l|xargs sed -r -i "s/conf.set\('(HAVE|ENABLE)_/conf.set10('\1_/"
$ git grep -Ee '#ifn?def (HAVE|ENABLE)' -l|xargs sed -r -i 's/#ifdef (HAVE|ENABLE)/#if \1/; s/#ifndef (HAVE|ENABLE)/#if ! \1/;'
$ git grep -Ee 'if.*defined\(HAVE' -l|xargs sed -i -r 's/defined\((HAVE_[A-Z0-9_]*)\)/\1/g'
$ git grep -Ee 'if.*defined\(ENABLE' -l|xargs sed -i -r 's/defined\((ENABLE_[A-Z0-9_]*)\)/\1/g'
+ manual changes to meson.build
squash! build-sys: use #if Y instead of #ifdef Y everywhere
v2:
- fix incorrect setting of HAVE_LIBIDN2
As suggested by CODING_STYLE we should use "void*" as type for generic memory,
and uint8_t* for generic bytes. Hence use that instead of "char*", which should
really be used only for strings these days.
Rather than passing a pointer to return the result, return it directly
from the function calls.
Also, return the result in native endianess, and let the callers care
about the conversion. For hash tables and bloom filters, we don't care,
but in order to keep MAC addresses and DHCP client IDs stable, we
explicitly convert to LE.
Change the "out" parameter from uint8_t[8] to uint64_t. On architectures which
enforce pointer alignment this fixes crashes when we previously cast an
unaligned array to uint64_t*, and on others this should at least improve
performance as the compiler now aligns these properly.
This also simplifies the code in most cases by getting rid of typecasts. The
only place which we can't change is struct duid's en.id, as that is _packed_
and public API, so we can't enforce alignment of the "id" field and have to
use memcpy instead.
Make the API of the new helpers more similar to the old wrapper.
In particular we now return the hash as a byte string to avoid
any endianness problems.
Make sure all variable-length inputs are properly terminated or that
their length is encoded in some way. This avoids ambiguity of
adjacent inputs.
E.g., in case of a hash function taking two strings, compressing "ab"
followed by "c" is now distinct from "a" followed by "bc".
All our hash functions are based on siphash24(), factor out
siphash_init() and siphash24_finalize() and pass the siphash
state to the hash functions rather than the hash key.
This simplifies the hash functions, and in particular makes
composition simpler as calling siphash24_compress() repeatedly
on separate chunks of input has the same effect as first
concatenating the input and then calling siphash23_compress()
on the result.
This may be used from multi-threaded programs (say through nss-resolve),
so we must protect the global list.
This is still only relevant for debug builds, so we do not try to handle
cases where the locking fail, but simply assert.
Currently, the HASHMAP iterators stop at the first NULL entry in a
hashmap. This is non-obvious and breaks users like sd-device, which
legitimately store NULL values in a hashmap.
Fix all the iterators by taking a pointer to the value storage, instead of
returning it. The iterators now return a boolean that tells whether the
end of the list was reached.
Current users of HASHMAP_FOREACH() are *NOT* changed to explicitly check
for NULL. If it turns out, there were users that inserted NULL into
hashmaps, but didn't properly check for it during iteration, then we
really want to find those and fix them.