From 552cf64a706c2bfe8bfc075694e2a7d5a8fd4758 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 9 Sep 2024 13:38:31 +0200 Subject: [PATCH 1/3] Replace isc_mem_destroy() with isc_mem_detach() Remove legacy isc_mem_destroy() and just use isc_mem_detach() as isc_mem_destroy() doesn't play well with call_rcu API. --- bin/check/named-checkconf.c | 2 +- bin/check/named-checkzone.c | 2 +- bin/confgen/rndc-confgen.c | 2 +- bin/confgen/tsig-keygen.c | 2 +- bin/dnssec/dnssec-cds.c | 2 +- bin/dnssec/dnssec-dsfromkey.c | 2 +- bin/dnssec/dnssec-importkey.c | 2 +- bin/dnssec/dnssec-keyfromlabel.c | 2 +- bin/dnssec/dnssec-keygen.c | 2 +- bin/dnssec/dnssec-revoke.c | 2 +- bin/dnssec/dnssec-settime.c | 2 +- bin/dnssec/dnssec-verify.c | 2 +- bin/tests/system/makejournal.c | 2 +- bin/tests/system/rsabigexponent/bigkey.c | 2 +- bin/tests/wire_test.c | 2 +- bin/tools/dnstap-read.c | 2 +- bin/tools/named-rrchecker.c | 2 +- doc/misc/cfg_test.c | 2 +- fuzz/dns_master_load.c | 2 +- fuzz/dns_qp.c | 2 +- fuzz/dns_rdata_fromtext.c | 2 +- lib/dns/dst_api.c | 2 +- lib/isc/crypto.c | 2 +- lib/isc/include/isc/mem.h | 7 ----- lib/isc/managers.c | 2 +- lib/isc/mem.c | 34 ------------------------ lib/isc/uv.c | 2 +- lib/isc/xml.c | 2 +- tests/bench/compress.c | 2 +- tests/bench/qpmulti.c | 2 +- tests/dns/qpdb_test.c | 4 +-- tests/include/tests/isc.h | 2 +- tests/isc/mem_test.c | 10 +++---- tests/libtest/isc.c | 2 +- 34 files changed, 37 insertions(+), 78 deletions(-) diff --git a/bin/check/named-checkconf.c b/bin/check/named-checkconf.c index e0210cfbd3..abdd78e8e4 100644 --- a/bin/check/named-checkconf.c +++ b/bin/check/named-checkconf.c @@ -761,7 +761,7 @@ cleanup: } if (mctx != NULL) { - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); } return result == ISC_R_SUCCESS ? 0 : 1; diff --git a/bin/check/named-checkzone.c b/bin/check/named-checkzone.c index 998171662c..fb49f842d6 100644 --- a/bin/check/named-checkzone.c +++ b/bin/check/named-checkzone.c @@ -577,7 +577,7 @@ main(int argc, char **argv) { fprintf(errout, "OK\n"); } destroy(); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return (result == ISC_R_SUCCESS) ? 0 : 1; } diff --git a/bin/confgen/rndc-confgen.c b/bin/confgen/rndc-confgen.c index a6526268b4..6f6fce3913 100644 --- a/bin/confgen/rndc-confgen.c +++ b/bin/confgen/rndc-confgen.c @@ -290,7 +290,7 @@ options {\n\ isc_mem_stats(mctx, stderr); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/bin/confgen/tsig-keygen.c b/bin/confgen/tsig-keygen.c index ee6470d9e9..ff0195b97b 100644 --- a/bin/confgen/tsig-keygen.c +++ b/bin/confgen/tsig-keygen.c @@ -296,7 +296,7 @@ nsupdate -k \n"); isc_mem_stats(mctx, stderr); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/bin/dnssec/dnssec-cds.c b/bin/dnssec/dnssec-cds.c index 13368126ef..d4993051d9 100644 --- a/bin/dnssec/dnssec-cds.c +++ b/bin/dnssec/dnssec-cds.c @@ -1075,7 +1075,7 @@ cleanup(void) { if (print_mem_stats && verbose > 10) { isc_mem_stats(mctx, stdout); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); } } diff --git a/bin/dnssec/dnssec-dsfromkey.c b/bin/dnssec/dnssec-dsfromkey.c index 9647638eac..c5c5c4d417 100644 --- a/bin/dnssec/dnssec-dsfromkey.c +++ b/bin/dnssec/dnssec-dsfromkey.c @@ -543,7 +543,7 @@ main(int argc, char **argv) { if (verbose > 10) { isc_mem_stats(mctx, stdout); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); fflush(stdout); if (ferror(stdout)) { diff --git a/bin/dnssec/dnssec-importkey.c b/bin/dnssec/dnssec-importkey.c index 3d5ca6724e..7ad8a14428 100644 --- a/bin/dnssec/dnssec-importkey.c +++ b/bin/dnssec/dnssec-importkey.c @@ -456,7 +456,7 @@ main(int argc, char **argv) { if (verbose > 10) { isc_mem_stats(mctx, stdout); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); fflush(stdout); if (ferror(stdout)) { diff --git a/bin/dnssec/dnssec-keyfromlabel.c b/bin/dnssec/dnssec-keyfromlabel.c index dadf6e0c9b..b8289358b2 100644 --- a/bin/dnssec/dnssec-keyfromlabel.c +++ b/bin/dnssec/dnssec-keyfromlabel.c @@ -746,7 +746,7 @@ main(int argc, char **argv) { isc_mem_stats(mctx, stdout); } isc_mem_free(mctx, label); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); if (freeit != NULL) { free(freeit); diff --git a/bin/dnssec/dnssec-keygen.c b/bin/dnssec/dnssec-keygen.c index adc98f07be..93d9d693a9 100644 --- a/bin/dnssec/dnssec-keygen.c +++ b/bin/dnssec/dnssec-keygen.c @@ -1280,7 +1280,7 @@ main(int argc, char **argv) { if (verbose > 10) { isc_mem_stats(mctx, stdout); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); if (freeit != NULL) { free(freeit); diff --git a/bin/dnssec/dnssec-revoke.c b/bin/dnssec/dnssec-revoke.c index 5f54ad297c..8c647fefd6 100644 --- a/bin/dnssec/dnssec-revoke.c +++ b/bin/dnssec/dnssec-revoke.c @@ -248,7 +248,7 @@ cleanup: if (dir != NULL) { isc_mem_free(mctx, dir); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/bin/dnssec/dnssec-settime.c b/bin/dnssec/dnssec-settime.c index 7f14e0fc38..cb99e54066 100644 --- a/bin/dnssec/dnssec-settime.c +++ b/bin/dnssec/dnssec-settime.c @@ -949,7 +949,7 @@ main(int argc, char **argv) { isc_mem_stats(mctx, stdout); } isc_mem_free(mctx, directory); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/bin/dnssec/dnssec-verify.c b/bin/dnssec/dnssec-verify.c index 1b00812c64..cd29431223 100644 --- a/bin/dnssec/dnssec-verify.c +++ b/bin/dnssec/dnssec-verify.c @@ -330,7 +330,7 @@ main(int argc, char *argv[]) { if (verbose > 10) { isc_mem_stats(mctx, stdout); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return result == ISC_R_SUCCESS ? 0 : 1; } diff --git a/bin/tests/system/makejournal.c b/bin/tests/system/makejournal.c index 94a92b9a73..eb85ad25a5 100644 --- a/bin/tests/system/makejournal.c +++ b/bin/tests/system/makejournal.c @@ -111,7 +111,7 @@ cleanup: } if (mctx != NULL) { - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); } return result != ISC_R_SUCCESS ? 1 : 0; diff --git a/bin/tests/system/rsabigexponent/bigkey.c b/bin/tests/system/rsabigexponent/bigkey.c index 2ae0f3dec0..57072297d0 100644 --- a/bin/tests/system/rsabigexponent/bigkey.c +++ b/bin/tests/system/rsabigexponent/bigkey.c @@ -140,7 +140,7 @@ main(int argc, char **argv) { printf("%s\n", filename); dst_key_free(&key); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/bin/tests/wire_test.c b/bin/tests/wire_test.c index bf130c7fc3..ec0ecccf5e 100644 --- a/bin/tests/wire_test.c +++ b/bin/tests/wire_test.c @@ -260,7 +260,7 @@ main(int argc, char *argv[]) { if (printmemstats) { isc_mem_stats(mctx, stdout); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/bin/tools/dnstap-read.c b/bin/tools/dnstap-read.c index 501d70ad96..c9e5406fc3 100644 --- a/bin/tools/dnstap-read.c +++ b/bin/tools/dnstap-read.c @@ -425,7 +425,7 @@ cleanup: if (message != NULL) { dns_message_detach(&message); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); exit(rv); } diff --git a/bin/tools/named-rrchecker.c b/bin/tools/named-rrchecker.c index 2de1d31bf8..9772bb1a8d 100644 --- a/bin/tools/named-rrchecker.c +++ b/bin/tools/named-rrchecker.c @@ -61,7 +61,7 @@ cleanup(void) { isc_lex_destroy(&lex); } if (mctx != NULL) { - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); } } diff --git a/doc/misc/cfg_test.c b/doc/misc/cfg_test.c index 64c64d2700..9a26acc194 100644 --- a/doc/misc/cfg_test.c +++ b/doc/misc/cfg_test.c @@ -154,7 +154,7 @@ main(int argc, char **argv) { if (memstats) { isc_mem_stats(mctx, stderr); } - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); fflush(stdout); if (ferror(stdout)) { diff --git a/fuzz/dns_master_load.c b/fuzz/dns_master_load.c index 1acd4c816a..09717329fa 100644 --- a/fuzz/dns_master_load.c +++ b/fuzz/dns_master_load.c @@ -74,6 +74,6 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { end: dns_db_detach(&db); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/fuzz/dns_qp.c b/fuzz/dns_qp.c index 1077013bfa..8b9fc08a6e 100644 --- a/fuzz/dns_qp.c +++ b/fuzz/dns_qp.c @@ -211,7 +211,7 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { } dns_qp_destroy(&qp); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); isc_mem_checkdestroyed(stderr); for (size_t i = 0; i < ARRAY_SIZE(item); i++) { diff --git a/fuzz/dns_rdata_fromtext.c b/fuzz/dns_rdata_fromtext.c index ecdcb3faca..24d3788dfe 100644 --- a/fuzz/dns_rdata_fromtext.c +++ b/fuzz/dns_rdata_fromtext.c @@ -145,6 +145,6 @@ LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { cleanup: isc_lex_close(lex); isc_lex_destroy(&lex); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c index 6446de1f6a..3f844264c6 100644 --- a/lib/dns/dst_api.c +++ b/lib/dns/dst_api.c @@ -226,7 +226,7 @@ dst__lib_shutdown(void) { } } - isc_mem_destroy(&dst__mctx); + isc_mem_detach(&dst__mctx); } bool diff --git a/lib/isc/crypto.c b/lib/isc/crypto.c index 209f49e90c..c99592befc 100644 --- a/lib/isc/crypto.c +++ b/lib/isc/crypto.c @@ -314,5 +314,5 @@ isc__crypto_shutdown(void) { OPENSSL_cleanup(); - isc_mem_destroy(&isc__crypto_mctx); + isc_mem_detach(&isc__crypto_mctx); } diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 28bdd64041..ad3b4bd97a 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -272,13 +272,6 @@ isc__mem_detach(isc_mem_t **_ISC_MEM_FLARG); */ /*@}*/ -#define isc_mem_destroy(cp) isc__mem_destroy((cp)_ISC_MEM_FILELINE) -void -isc__mem_destroy(isc_mem_t **_ISC_MEM_FLARG); -/*%< - * Destroy a memory context. - */ - void isc_mem_stats(isc_mem_t *mctx, FILE *out); /*%< diff --git a/lib/isc/managers.c b/lib/isc/managers.c index 5471122300..b4912e8ac0 100644 --- a/lib/isc/managers.c +++ b/lib/isc/managers.c @@ -47,5 +47,5 @@ isc_managers_destroy(isc_mem_t **mctxp, isc_loopmgr_t **loopmgrp, isc_netmgr_destroy(netmgrp); isc_loopmgr_destroy(loopmgrp); - isc_mem_destroy(mctxp); + isc_mem_detach(mctxp); } diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 2651c503ab..4fb27ad23d 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -598,40 +598,6 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size, isc__mem_detach(&ctx FLARG_PASS); } -void -isc__mem_destroy(isc_mem_t **ctxp FLARG) { - isc_mem_t *ctx = NULL; - - /* - * This routine provides legacy support for callers who use mctxs - * without attaching/detaching. - */ - - REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); - - ctx = *ctxp; - *ctxp = NULL; - - rcu_barrier(); - -#if ISC_MEM_TRACKLINES - if ((ctx->debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, "destroy mctx %p file %s line %u\n", ctx, file, - line); - } - - if (isc_refcount_decrement(&ctx->references) > 1) { - print_active(ctx, stderr); - } -#else /* if ISC_MEM_TRACKLINES */ - isc_refcount_decrementz(&ctx->references); -#endif /* if ISC_MEM_TRACKLINES */ - isc_refcount_destroy(&ctx->references); - destroy(ctx); - - *ctxp = NULL; -} - void * isc__mem_get(isc_mem_t *ctx, size_t size, int flags FLARG) { void *ptr = NULL; diff --git a/lib/isc/uv.c b/lib/isc/uv.c index 7ad878cf7f..431cecbf48 100644 --- a/lib/isc/uv.c +++ b/lib/isc/uv.c @@ -146,7 +146,7 @@ void isc__uv_shutdown(void) { #if UV_VERSION_HEX >= UV_VERSION(1, 38, 0) uv_library_shutdown(); - isc_mem_destroy(&isc__uv_mctx); + isc_mem_detach(&isc__uv_mctx); #endif /* UV_VERSION_HEX < UV_VERSION(1, 38, 0) */ } diff --git a/lib/isc/xml.c b/lib/isc/xml.c index 7dd9424fe5..cbff418904 100644 --- a/lib/isc/xml.c +++ b/lib/isc/xml.c @@ -64,7 +64,7 @@ void isc__xml_shutdown(void) { #ifdef HAVE_LIBXML2 xmlCleanupParser(); - isc_mem_destroy(&isc__xml_mctx); + isc_mem_detach(&isc__xml_mctx); #endif /* HAVE_LIBXML2 */ } diff --git a/tests/bench/compress.c b/tests/bench/compress.c index 912e8847e2..f6c51afea1 100644 --- a/tests/bench/compress.c +++ b/tests/bench/compress.c @@ -100,7 +100,7 @@ main(void) { printf("names %u\n", count); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/tests/bench/qpmulti.c b/tests/bench/qpmulti.c index 13aded4098..d2d5d5f8b7 100644 --- a/tests/bench/qpmulti.c +++ b/tests/bench/qpmulti.c @@ -889,7 +889,7 @@ main(void) { isc_mem_free(mctx, item); isc_mem_checkdestroyed(stdout); - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } diff --git a/tests/dns/qpdb_test.c b/tests/dns/qpdb_test.c index 73931889df..24101becb9 100644 --- a/tests/dns/qpdb_test.c +++ b/tests/dns/qpdb_test.c @@ -158,7 +158,7 @@ ISC_LOOP_TEST_IMPL(overmempurge_bigrdata) { } dns_db_detach(&db); - isc_mem_destroy(&mctx2); + isc_mem_detach(&mctx2); isc_loopmgr_shutdown(loopmgr); } @@ -208,7 +208,7 @@ ISC_LOOP_TEST_IMPL(overmempurge_longname) { } dns_db_detach(&db); - isc_mem_destroy(&mctx2); + isc_mem_detach(&mctx2); isc_loopmgr_shutdown(loopmgr); } diff --git a/tests/include/tests/isc.h b/tests/include/tests/isc.h index b49b509518..a7464d8019 100644 --- a/tests/include/tests/isc.h +++ b/tests/include/tests/isc.h @@ -249,7 +249,7 @@ teardown_managers(void **state); r = cmocka_run_group_tests(tests, setup, teardown); \ } \ \ - isc_mem_destroy(&mctx); \ + isc_mem_detach(&mctx); \ \ return (r); \ } diff --git a/tests/isc/mem_test.c b/tests/isc/mem_test.c index ddb655440d..aaf94c0d77 100644 --- a/tests/isc/mem_test.c +++ b/tests/isc/mem_test.c @@ -197,7 +197,7 @@ ISC_RUN_TEST_IMPL(isc_mem_inuse) { assert_int_equal(diff, 0); - isc_mem_destroy(&mctx2); + isc_mem_detach(&mctx2); } ISC_RUN_TEST_IMPL(isc_mem_zeroget) { @@ -323,7 +323,7 @@ ISC_RUN_TEST_IMPL(isc_mem_overmem) { isc_mem_free(omctx, data1); assert_false(isc_mem_isovermem(omctx)); - isc_mem_destroy(&omctx); + isc_mem_detach(&omctx); } #if ISC_MEM_TRACKLINES @@ -345,7 +345,7 @@ ISC_RUN_TEST_IMPL(isc_mem_noflags) { assert_non_null(ptr); isc__mem_printactive(mctx2, f); isc_mem_put(mctx2, ptr, 2048); - isc_mem_destroy(&mctx2); + isc_mem_detach(&mctx2); isc_mem_debugging = ISC_MEM_DEBUGRECORD; isc_stdio_close(f); @@ -379,7 +379,7 @@ ISC_RUN_TEST_IMPL(isc_mem_recordflag) { assert_non_null(ptr); isc__mem_printactive(mctx2, f); isc_mem_put(mctx2, ptr, 2048); - isc_mem_destroy(&mctx2); + isc_mem_detach(&mctx2); isc_stdio_close(f); memset(buf, 0, sizeof(buf)); @@ -419,7 +419,7 @@ ISC_RUN_TEST_IMPL(isc_mem_traceflag) { assert_non_null(ptr); isc__mem_printactive(mctx2, f); isc_mem_put(mctx2, ptr, 2048); - isc_mem_destroy(&mctx2); + isc_mem_detach(&mctx2); isc_mem_debugging = ISC_MEM_DEBUGRECORD; isc_stdio_close(f); diff --git a/tests/libtest/isc.c b/tests/libtest/isc.c index c53cfff24b..92a3d8afac 100644 --- a/tests/libtest/isc.c +++ b/tests/libtest/isc.c @@ -79,7 +79,7 @@ setup_mctx(void **state ISC_ATTR_UNUSED) { int teardown_mctx(void **state ISC_ATTR_UNUSED) { - isc_mem_destroy(&mctx); + isc_mem_detach(&mctx); return 0; } From eab9fc22e70041e1d8954bef9fd5e61e4948b269 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 9 Sep 2024 12:14:05 +0200 Subject: [PATCH 2/3] Replace attach/detach in isc_mem with refcount implementation The isc_mem API is one of the most commonly used APIs that didn't used ISC_REFCOUNT_DECL and ISC_REFCOUNT_IMPL macros. Replace the implementation of isc_mem_attach(), isc_mem_detach() and isc_mem_destroy() with the respective macros. This also removes the legacy isc_mem_destroy() functionality that would check whether all references had been detached from the memory context as it doesn't work reliably when using the call_rcu() API. Instead of doing this individually, call isc_mem_checkdestroyed(stderr) from the isc_mem_destroy() macro to keep the extra check that all contexts were freed when the program is exiting. --- lib/isc/include/isc/mem.h | 35 +++++++++++------------------ lib/isc/mem.c | 46 +++++++++++++-------------------------- 2 files changed, 28 insertions(+), 53 deletions(-) diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index ad3b4bd97a..9f44628d01 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -21,9 +21,12 @@ #include #include #include +#include #include #include +/* Add -DISC_MEM_TRACE=1 to CFLAGS for detailed reference tracing */ + /*% * Define ISC_MEM_TRACKLINES=1 to turn on detailed tracing of memory * allocation and freeing by file and line number. @@ -249,28 +252,16 @@ isc_mem_arena_set_dirty_decay_ms(isc_mem_t *mctx, const ssize_t decay_ms); */ /*@}*/ -void -isc_mem_attach(isc_mem_t *, isc_mem_t **); - -/*@{*/ -void -isc_mem_attach(isc_mem_t *, isc_mem_t **); -#define isc_mem_detach(cp) isc__mem_detach((cp)_ISC_MEM_FILELINE) -void -isc__mem_detach(isc_mem_t **_ISC_MEM_FLARG); -/*!< - * \brief Attach to / detach from a memory context. - * - * This is intended for applications that use multiple memory contexts - * in such a way that it is not obvious when the last allocations from - * a given context has been freed and destroying the context is safe. - * - * Most applications do not need to call these functions as they can - * simply create a single memory context at the beginning of main() - * and destroy it at the end of main(), thereby guaranteeing that it - * is not destroyed while there are outstanding allocations. - */ -/*@}*/ +#if ISC_MEM_TRACE +#define isc_mem_ref(ptr) isc_mem__ref(ptr, __func__, __FILE__, __LINE__) +#define isc_mem_unref(ptr) isc_mem__unref(ptr, __func__, __FILE__, __LINE__) +#define isc_mem_attach(ptr, ptrp) \ + isc_mem__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define isc_mem_detach(ptrp) isc_mem__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(isc_mem); +#else +ISC_REFCOUNT_DECL(isc_mem); +#endif void isc_mem_stats(isc_mem_t *mctx, FILE *out); diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 4fb27ad23d..5edf15a8f6 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -427,6 +427,8 @@ void isc__mem_shutdown(void) { bool empty; + rcu_barrier(); + isc__mem_checkdestroyed(); LOCK(&contextslock); @@ -495,8 +497,11 @@ mem_create(isc_mem_t **ctxp, unsigned int debugging, unsigned int flags, */ static void -destroy(isc_mem_t *ctx) { +mem_destroy(isc_mem_t *ctx) { unsigned int arena_no; + + isc_refcount_destroy(&ctx->references); + LOCK(&contextslock); ISC_LIST_UNLINK(contexts, ctx, link); UNLOCK(&contextslock); @@ -543,36 +548,11 @@ destroy(isc_mem_t *ctx) { } } -void -isc_mem_attach(isc_mem_t *source, isc_mem_t **targetp) { - REQUIRE(VALID_CONTEXT(source)); - REQUIRE(targetp != NULL && *targetp == NULL); - - isc_refcount_increment(&source->references); - - *targetp = source; -} - -void -isc__mem_detach(isc_mem_t **ctxp FLARG) { - isc_mem_t *ctx = NULL; - - REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp)); - - ctx = *ctxp; - *ctxp = NULL; - - if (isc_refcount_decrement(&ctx->references) == 1) { - isc_refcount_destroy(&ctx->references); -#if ISC_MEM_TRACKLINES - if ((ctx->debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, "destroy mctx %p file %s line %u\n", - ctx, file, line); - } +#if ISC_MEM_TRACE +ISC_REFCOUNT_TRACE_IMPL(isc_mem, mem_destroy); +#else +ISC_REFCOUNT_IMPL(isc_mem, mem_destroy); #endif - destroy(ctx); - } -} /* * isc_mem_putanddetach() is the equivalent of: @@ -595,7 +575,11 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size, *ctxp = NULL; isc__mem_put(ctx, ptr, size, flags FLARG_PASS); - isc__mem_detach(&ctx FLARG_PASS); +#if ISC_MEM_TRACE + isc_mem__detach(&ctx, __func__, file, line); +#else + isc_mem_detach(&ctx); +#endif } void * From 1fae6ccea13773cfcc066d058a53aa57ec0f9a32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 9 Sep 2024 13:59:43 +0200 Subject: [PATCH 3/3] Add the call function tracking to isc_mem API As we already track __func__, __FILE__, __LINE__ triplet in most places, add the function tracking to the isc_mem tracking API. --- lib/isc/crypto.c | 8 ++-- lib/isc/include/isc/mem.h | 4 +- lib/isc/mem.c | 77 +++++++++++++++++++++------------------ util/memleak.pl | 4 +- 4 files changed, 49 insertions(+), 44 deletions(-) diff --git a/lib/isc/crypto.c b/lib/isc/crypto.c index c99592befc..604a6439b7 100644 --- a/lib/isc/crypto.c +++ b/lib/isc/crypto.c @@ -114,14 +114,14 @@ unregister_algorithms(void) { static void * isc__crypto_malloc_ex(size_t size, const char *file, int line) { - return isc__mem_allocate(isc__crypto_mctx, size, 0, file, + return isc__mem_allocate(isc__crypto_mctx, size, 0, __func__, file, (unsigned int)line); } static void * isc__crypto_realloc_ex(void *ptr, size_t size, const char *file, int line) { - return isc__mem_reallocate(isc__crypto_mctx, ptr, size, 0, file, - (unsigned int)line); + return isc__mem_reallocate(isc__crypto_mctx, ptr, size, 0, __func__, + file, (unsigned int)line); } static void @@ -130,7 +130,7 @@ isc__crypto_free_ex(void *ptr, const char *file, int line) { return; } if (isc__crypto_mctx != NULL) { - isc__mem_free(isc__crypto_mctx, ptr, 0, file, + isc__mem_free(isc__crypto_mctx, ptr, 0, __func__, file, (unsigned int)line); } } diff --git a/lib/isc/include/isc/mem.h b/lib/isc/include/isc/mem.h index 9f44628d01..09382cfc02 100644 --- a/lib/isc/include/isc/mem.h +++ b/lib/isc/include/isc/mem.h @@ -65,8 +65,8 @@ extern unsigned int isc_mem_defaultflags; /*@}*/ #if ISC_MEM_TRACKLINES -#define _ISC_MEM_FILELINE , __FILE__, __LINE__ -#define _ISC_MEM_FLARG , const char *, unsigned int +#define _ISC_MEM_FILELINE , __func__, __FILE__, __LINE__ +#define _ISC_MEM_FLARG , const char *, const char *, unsigned int #else /* if ISC_MEM_TRACKLINES */ #define _ISC_MEM_FILELINE #define _ISC_MEM_FLARG diff --git a/lib/isc/mem.c b/lib/isc/mem.c index 5edf15a8f6..1b569f042d 100644 --- a/lib/isc/mem.c +++ b/lib/isc/mem.c @@ -86,14 +86,15 @@ struct debuglink { ISC_LINK(debuglink_t) link; const void *ptr; size_t size; + const char *func; const char *file; unsigned int line; }; typedef ISC_LIST(debuglink_t) debuglist_t; -#define FLARG_PASS , file, line -#define FLARG , const char *file, unsigned int line +#define FLARG_PASS , func, file, line +#define FLARG , const char *func, const char *file, unsigned int line #else /* if ISC_MEM_TRACKLINES */ #define FLARG_PASS #define FLARG @@ -164,8 +165,8 @@ struct isc_mempool { */ #if !ISC_MEM_TRACKLINES -#define ADD_TRACE(mctx, ptr, size, file, line) -#define DELETE_TRACE(mctx, ptr, size, file, line) +#define ADD_TRACE(mctx, ptr, size, func, file, line) +#define DELETE_TRACE(mctx, ptr, size, func, file, line) #define ISC_MEMFUNC_SCOPE #else /* if !ISC_MEM_TRACKLINES */ #define TRACE_OR_RECORD (ISC_MEM_DEBUGTRACE | ISC_MEM_DEBUGRECORD) @@ -173,14 +174,14 @@ struct isc_mempool { #define SHOULD_TRACE_OR_RECORD(mctx, ptr) \ (((mctx)->debugging & TRACE_OR_RECORD) != 0 && ptr != NULL) -#define ADD_TRACE(mctx, ptr, size, file, line) \ - if (SHOULD_TRACE_OR_RECORD(mctx, ptr)) { \ - add_trace_entry(mctx, ptr, size, file, line); \ +#define ADD_TRACE(mctx, ptr, size, func, file, line) \ + if (SHOULD_TRACE_OR_RECORD(mctx, ptr)) { \ + add_trace_entry(mctx, ptr, size, func, file, line); \ } -#define DELETE_TRACE(mctx, ptr, size, file, line) \ - if (SHOULD_TRACE_OR_RECORD(mctx, ptr)) { \ - delete_trace_entry(mctx, ptr, size, file, line); \ +#define DELETE_TRACE(mctx, ptr, size, func, file, line) \ + if (SHOULD_TRACE_OR_RECORD(mctx, ptr)) { \ + delete_trace_entry(mctx, ptr, size, func, file, line); \ } static void @@ -200,8 +201,9 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { MCTXLOCK(mctx); if ((mctx->debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, "add %p size %zu file %s line %u mctx %p\n", - ptr, size, file, line, mctx); + fprintf(stderr, + "add %p size %zu func %s file %s line %u mctx %p\n", + ptr, size, func, file, line, mctx); } if (mctx->debuglist == NULL) { @@ -225,6 +227,7 @@ add_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { ISC_LINK_INIT(dl, link); dl->ptr = ptr; dl->size = size; + dl->func = func; dl->file = file; dl->line = line; @@ -235,8 +238,7 @@ unlock: } static void -delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, - const char *file, unsigned int line) { +delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size FLARG) { debuglink_t *dl = NULL; uint32_t hash; uint32_t idx; @@ -244,8 +246,9 @@ delete_trace_entry(isc_mem_t *mctx, const void *ptr, size_t size, MCTXLOCK(mctx); if ((mctx->debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, "del %p size %zu file %s line %u mctx %p\n", - ptr, size, file, line, mctx); + fprintf(stderr, + "del %p size %zu func %s file %s line %u mctx %p\n", + ptr, size, func, file, line, mctx); } if (mctx->debuglist == NULL) { @@ -576,7 +579,7 @@ isc__mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size, isc__mem_put(ctx, ptr, size, flags FLARG_PASS); #if ISC_MEM_TRACE - isc_mem__detach(&ctx, __func__, file, line); + isc_mem__detach(&ctx, func, file, line); #else isc_mem_detach(&ctx); #endif @@ -591,7 +594,7 @@ isc__mem_get(isc_mem_t *ctx, size_t size, int flags FLARG) { ptr = mem_get(ctx, size, flags); mem_getstats(ctx, size); - ADD_TRACE(ctx, ptr, size, file, line); + ADD_TRACE(ctx, ptr, size, func, file, line); return ptr; } @@ -600,7 +603,7 @@ void isc__mem_put(isc_mem_t *ctx, void *ptr, size_t size, int flags FLARG) { REQUIRE(VALID_CONTEXT(ctx)); - DELETE_TRACE(ctx, ptr, size, file, line); + DELETE_TRACE(ctx, ptr, size, func, file, line); mem_putstats(ctx, size); mem_put(ctx, ptr, size, flags); @@ -697,7 +700,7 @@ isc__mem_allocate(isc_mem_t *ctx, size_t size, int flags FLARG) { size = sallocx(ptr, flags | ctx->jemalloc_flags); mem_getstats(ctx, size); - ADD_TRACE(ctx, ptr, size, file, line); + ADD_TRACE(ctx, ptr, size, func, file, line); return ptr; } @@ -713,13 +716,13 @@ isc__mem_reget(isc_mem_t *ctx, void *old_ptr, size_t old_size, size_t new_size, } else if (new_size == 0) { isc__mem_put(ctx, old_ptr, old_size, flags FLARG_PASS); } else { - DELETE_TRACE(ctx, old_ptr, old_size, file, line); + DELETE_TRACE(ctx, old_ptr, old_size, func, file, line); mem_putstats(ctx, old_size); new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size, flags); mem_getstats(ctx, new_size); - ADD_TRACE(ctx, new_ptr, new_size, file, line); + ADD_TRACE(ctx, new_ptr, new_size, func, file, line); /* * We want to postpone the call to water in edge case @@ -745,7 +748,7 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size, } else { size_t old_size = sallocx(old_ptr, flags | ctx->jemalloc_flags); - DELETE_TRACE(ctx, old_ptr, old_size, file, line); + DELETE_TRACE(ctx, old_ptr, old_size, func, file, line); mem_putstats(ctx, old_size); new_ptr = mem_realloc(ctx, old_ptr, old_size, new_size, flags); @@ -754,7 +757,7 @@ isc__mem_reallocate(isc_mem_t *ctx, void *old_ptr, size_t new_size, new_size = sallocx(new_ptr, flags | ctx->jemalloc_flags); mem_getstats(ctx, new_size); - ADD_TRACE(ctx, new_ptr, new_size, file, line); + ADD_TRACE(ctx, new_ptr, new_size, func, file, line); /* * We want to postpone the call to water in edge case @@ -775,7 +778,7 @@ isc__mem_free(isc_mem_t *ctx, void *ptr, int flags FLARG) { size = sallocx(ptr, flags | ctx->jemalloc_flags); - DELETE_TRACE(ctx, ptr, size, file, line); + DELETE_TRACE(ctx, ptr, size, func, file, line); mem_putstats(ctx, size); mem_put(ctx, ptr, size, flags); @@ -960,8 +963,9 @@ isc__mempool_create(isc_mem_t *restrict mctx, const size_t element_size, #if ISC_MEM_TRACKLINES if ((mctx->debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, "create pool %p file %s line %u mctx %p\n", - mpctx, file, line, mctx); + fprintf(stderr, + "create pool %p func %s file %s line %u mctx %p\n", + mpctx, func, file, line, mctx); } #endif /* ISC_MEM_TRACKLINES */ @@ -1000,8 +1004,9 @@ isc__mempool_destroy(isc_mempool_t **restrict mpctxp FLARG) { #if ISC_MEM_TRACKLINES if ((mctx->debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, "destroy pool %p file %s line %u mctx %p\n", - mpctx, file, line, mctx); + fprintf(stderr, + "destroy pool %p func %s file %s line %u mctx %p\n", + mpctx, func, file, line, mctx); } #endif @@ -1073,7 +1078,7 @@ isc__mempool_get(isc_mempool_t *restrict mpctx FLARG) { mpctx->freecount--; mpctx->gets++; - ADD_TRACE(mpctx->mctx, item, mpctx->size, file, line); + ADD_TRACE(mpctx->mctx, item, mpctx->size, func, file, line); return item; } @@ -1097,7 +1102,7 @@ isc__mempool_put(isc_mempool_t *restrict mpctx, void *mem FLARG) { INSIST(mpctx->allocated > 0); mpctx->allocated--; - DELETE_TRACE(mctx, mem, mpctx->size, file, line); + DELETE_TRACE(mctx, mem, mpctx->size, func, file, line); /* * If our free list is full, return this to the mctx directly. @@ -1432,8 +1437,8 @@ isc__mem_create(isc_mem_t **mctxp FLARG) { mem_create(mctxp, isc_mem_debugging, isc_mem_defaultflags, 0); #if ISC_MEM_TRACKLINES if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { - fprintf(stderr, "create mctx %p file %s line %u\n", *mctxp, - file, line); + fprintf(stderr, "create mctx %p func %s file %s line %u\n", + *mctxp, func, file, line); } #endif /* ISC_MEM_TRACKLINES */ } @@ -1459,9 +1464,9 @@ isc__mem_create_arena(isc_mem_t **mctxp FLARG) { #if ISC_MEM_TRACKLINES if ((isc_mem_debugging & ISC_MEM_DEBUGTRACE) != 0) { fprintf(stderr, - "create mctx %p file %s line %u for jemalloc arena " - "%u\n", - *mctxp, file, line, arena_no); + "create mctx %p func %s file %s line %u " + "for jemalloc arena %u\n", + *mctxp, func, file, line, arena_no); } #endif /* ISC_MEM_TRACKLINES */ } diff --git a/util/memleak.pl b/util/memleak.pl index 7e637d0a1d..d1db14539e 100644 --- a/util/memleak.pl +++ b/util/memleak.pl @@ -17,8 +17,8 @@ $mem_stats = ''; while (<>) { - $gets{$1.$2} = $_ if (/add (?:0x)?([0-9a-f]+) size (?:0x)?([0-9]+) file/); - delete $gets{$1.$2} if /del (?:0x)?([0-9a-f]+) size (?:0x)?([0-9]+) file/; + $gets{$1.$2} = $_ if (/add (?:0x)?([0-9a-f]+) size (?:0x)?([0-9]+) func/); + delete $gets{$1.$2} if /del (?:0x)?([0-9a-f]+) size (?:0x)?([0-9]+) func/; $mem_stats .= $_ if /\d+ gets, +(\d+) rem/ && $1 > 0; } print join('', values %gets);