From 633e9e8e6be84ea2186ba157c775cbfbbaed997c Mon Sep 17 00:00:00 2001 From: Vizonex Date: Sun, 27 Jul 2025 15:04:59 -0500 Subject: [PATCH 01/25] add test for memory leakage during deletion of value --- requirements/pytest.txt | 1 + tests/isolated/multidict_pop.py | 39 +++++++++++++++++++++++++++++++++ tests/test_leaks.py | 1 + 3 files changed, 41 insertions(+) create mode 100644 tests/isolated/multidict_pop.py diff --git a/requirements/pytest.txt b/requirements/pytest.txt index fd61e5d7e..b58422791 100644 --- a/requirements/pytest.txt +++ b/requirements/pytest.txt @@ -2,3 +2,4 @@ objgraph==3.6.2 pytest==8.4.0 pytest-codspeed==3.2.0 pytest-cov==6.1.0 +psutil==7.0.0 diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py new file mode 100644 index 000000000..34494e85a --- /dev/null +++ b/tests/isolated/multidict_pop.py @@ -0,0 +1,39 @@ +# Test for memory leaks surrounding deletion of values. +# SEE: https://github.com/aio-libs/multidict/issues/1232 + +import gc +import sys +import psutil +import os +from multidict import MultiDict + +def get_memory_usage(): + process = psutil.Process(os.getpid()) + memory_info = process.memory_info() + return memory_info.rss / (1024 * 1024) + +keys = [f"X-Any-{i}" for i in range(1000)] +headers = {key: key * 2 for key in keys} + + +def _run_isolated_case() -> None: + for _ in range(10): + for _ in range(1000): + result = MultiDict() + result.update(headers) + for k in keys: + result.pop(k) + # popitem() currently is unaffected but the others all have memory leaks... + # result.popone(k) + # result.popall(k) + del result + gc.collect() + usage = get_memory_usage() + # We should never go over 100MB + if usage > 100: + sys.exit(1) + sys.exit(0) + +if __name__ == "__main__": + _run_isolated_case() + diff --git a/tests/test_leaks.py b/tests/test_leaks.py index ded7cf065..7bc9e63d2 100644 --- a/tests/test_leaks.py +++ b/tests/test_leaks.py @@ -15,6 +15,7 @@ "multidict_extend_multidict.py", "multidict_extend_tuple.py", "multidict_update_multidict.py", + "multidict_pop.py" ), ) @pytest.mark.leaks From 8acea8112e9dcbc9aa63f18a0ff2afbbbe6e8c58 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sun, 27 Jul 2025 20:22:14 +0000 Subject: [PATCH 02/25] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/isolated/multidict_pop.py | 4 +++- tests/test_leaks.py | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 34494e85a..a0dfb7e8c 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -7,11 +7,13 @@ import os from multidict import MultiDict + def get_memory_usage(): process = psutil.Process(os.getpid()) memory_info = process.memory_info() return memory_info.rss / (1024 * 1024) + keys = [f"X-Any-{i}" for i in range(1000)] headers = {key: key * 2 for key in keys} @@ -34,6 +36,6 @@ def _run_isolated_case() -> None: sys.exit(1) sys.exit(0) + if __name__ == "__main__": _run_isolated_case() - diff --git a/tests/test_leaks.py b/tests/test_leaks.py index 7bc9e63d2..56126d4bc 100644 --- a/tests/test_leaks.py +++ b/tests/test_leaks.py @@ -15,7 +15,7 @@ "multidict_extend_multidict.py", "multidict_extend_tuple.py", "multidict_update_multidict.py", - "multidict_pop.py" + "multidict_pop.py", ), ) @pytest.mark.leaks From c06df7d22936f35797754062044da4d163a50d55 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Sun, 27 Jul 2025 19:17:01 -0500 Subject: [PATCH 03/25] trim memory before checking if memory had been leaked to prevent bougus assumptions. --- tests/isolated/multidict_pop.py | 109 ++++++++++++++++++++++++++++---- 1 file changed, 96 insertions(+), 13 deletions(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 34494e85a..c9cb2364c 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -1,12 +1,66 @@ # Test for memory leaks surrounding deletion of values. -# SEE: https://github.com/aio-libs/multidict/issues/1232 +# SEE: https://github.com/aio-libs/multidict/issues/1232 +# We want to make sure that bad predictions or bougus claims +# can be prevented in the future. import gc import sys import psutil import os +import ctypes from multidict import MultiDict +# Code was borrowed for testing with windows +# SEE: https://stackoverflow.com/a/79150440 + +def trim_windows_process_memory(pid: int = None) -> bool: + """Causes effect similar to malloc_trim on -nix.""" + + SIZE_T = ctypes.c_uint32 if ctypes.sizeof(ctypes.c_void_p) == 4 else ctypes.c_uint64 + + # Get a handle to the current process + if not pid: + pid = ctypes.windll.kernel32.GetCurrentProcess() + + # Sometimes FileNotFoundError can appear so the code was + # changed to handle that workaround. + + ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.argtypes = [ + ctypes.c_void_p, # Process handle + SIZE_T, # Minimum working set size + SIZE_T, # Maximum working set size + ctypes.c_ulong, # Flags + ] + ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.restype = ctypes.c_bool + + # Define constants for SetProcessWorkingSetSizeEx + QUOTA_LIMITS_HARDWS_MIN_DISABLE = 0x00000002 + + # Attempt to set the working set size + if not ctypes.windll.kernel32.SetProcessWorkingSetSizeEx(pid, SIZE_T(-1), SIZE_T(-1), QUOTA_LIMITS_HARDWS_MIN_DISABLE): + # Retrieve the error code + error_code = ctypes.windll.kernel32.GetLastError() + # let's print it since we aren't using a logger... + print(f"SetProcessWorkingSetSizeEx failed with error code: {error_code}") + return False + return True + + + +def trim_ram() -> None: + """Forces python garbage collection. + Most importantly, calls malloc_trim/SetProcessWorkingSetSizeEx, which fixes pandas/libc (?) memory leak.""" + + gc.collect() + if sys.platform == "win32": + assert trim_windows_process_memory(), "trim_ram failed" + else: + try: + ctypes.CDLL("libc.so.6").malloc_trim(0) + except Exception as e: + print(" attempt failed") + raise e + def get_memory_usage(): process = psutil.Process(os.getpid()) memory_info = process.memory_info() @@ -16,22 +70,51 @@ def get_memory_usage(): headers = {key: key * 2 for key in keys} -def _run_isolated_case() -> None: +def check_for_leak(): + trim_ram() + usage = get_memory_usage() + # We should never go over 100MB + if usage > 100: + sys.exit(1) + + +def _test_pop(): for _ in range(10): for _ in range(1000): - result = MultiDict() - result.update(headers) + result = MultiDict(headers) for k in keys: result.pop(k) - # popitem() currently is unaffected but the others all have memory leaks... - # result.popone(k) - # result.popall(k) - del result - gc.collect() - usage = get_memory_usage() - # We should never go over 100MB - if usage > 100: - sys.exit(1) + check_for_leak() + +def _test_popall(): + for _ in range(10): + for _ in range(1000): + result = MultiDict(headers) + for k in keys: + result.popall(k) + check_for_leak() + +def _test_popone(): + for _ in range(10): + for _ in range(1000): + result = MultiDict(headers) + for k in keys: + result.popone(k) + check_for_leak() + +def _test_del(): + for _ in range(10): + for _ in range(1000): + result = MultiDict(headers) + for k in keys: + del result[k] + check_for_leak() + +def _run_isolated_case() -> None: + _test_pop() + _test_popall() + _test_popone() + _test_del() sys.exit(0) if __name__ == "__main__": From a1fe938238509c74065c91232f44d060bb4891f9 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Sun, 27 Jul 2025 19:50:42 -0500 Subject: [PATCH 04/25] add contribution to timeline --- CHANGES/1233.contrib.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 CHANGES/1233.contrib.rst diff --git a/CHANGES/1233.contrib.rst b/CHANGES/1233.contrib.rst new file mode 100644 index 000000000..1ac7cce7d --- /dev/null +++ b/CHANGES/1233.contrib.rst @@ -0,0 +1,2 @@ +Added memory leak test for popping or deleting attributes from a multidict to prevent future issues or bougus claims. +-- by :user:`Vizonex` From 4ec11587044fb6c70507de030eb2e3c263b0bfc2 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Sun, 27 Jul 2025 20:01:24 -0500 Subject: [PATCH 05/25] merge the two functions together so that mypy will accept it. --- tests/isolated/multidict_pop.py | 69 +++++++++++++++------------------ 1 file changed, 32 insertions(+), 37 deletions(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 2562dd8b7..7a269b7ad 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -9,55 +9,50 @@ import os import ctypes from multidict import MultiDict -from typing import Optional # Code was borrowed for testing with windows and other operating systems respectively. # SEE: https://stackoverflow.com/a/79150440 -def trim_windows_process_memory(pid: Optional[int] = None) -> bool: - """Causes effect similar to malloc_trim on -nix.""" - - SIZE_T = ctypes.c_uint32 if ctypes.sizeof(ctypes.c_void_p) == 4 else ctypes.c_uint64 - - # Get a handle to the current process - if not pid: - pid = ctypes.windll.kernel32.GetCurrentProcess() - - # Sometimes FileNotFoundError can appear so the code was - # changed to handle that workaround. - - ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.argtypes = [ - ctypes.c_void_p, # Process handle - SIZE_T, # Minimum working set size - SIZE_T, # Maximum working set size - ctypes.c_ulong, # Flags - ] - ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.restype = ctypes.c_bool - - # Define constants for SetProcessWorkingSetSizeEx - QUOTA_LIMITS_HARDWS_MIN_DISABLE = 0x00000002 - - # Attempt to set the working set size - if not ctypes.windll.kernel32.SetProcessWorkingSetSizeEx( - pid, SIZE_T(-1), SIZE_T(-1), QUOTA_LIMITS_HARDWS_MIN_DISABLE - ): - # Retrieve the error code - error_code = ctypes.windll.kernel32.GetLastError() - # let's print it since we aren't using a logger... - print(f"SetProcessWorkingSetSizeEx failed with error code: {error_code}") - return False - return True - - def trim_ram() -> None: """Forces python garbage collection. Most importantly, calls malloc_trim/SetProcessWorkingSetSizeEx, which fixes pandas/libc (?) memory leak.""" gc.collect() if sys.platform == "win32": - assert trim_windows_process_memory(), "trim_ram failed" + SIZE_T = ( + ctypes.c_uint32 if ctypes.sizeof(ctypes.c_void_p) == 4 else ctypes.c_uint64 + ) + + # Get a handle to the current process + + pid = ctypes.windll.kernel32.GetCurrentProcess() + + # Sometimes FileNotFoundError can appear so the code was + # changed to handle that workaround. + + ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.argtypes = [ + ctypes.c_void_p, # Process handle + SIZE_T, # Minimum working set size + SIZE_T, # Maximum working set size + ctypes.c_ulong, # Flags + ] + ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.restype = ctypes.c_bool + + # Define constants for SetProcessWorkingSetSizeEx + QUOTA_LIMITS_HARDWS_MIN_DISABLE = 0x00000002 + + # Attempt to set the working set size + if not ctypes.windll.kernel32.SetProcessWorkingSetSizeEx( + pid, SIZE_T(-1), SIZE_T(-1), QUOTA_LIMITS_HARDWS_MIN_DISABLE + ): + # Retrieve the error code + error_code = ctypes.windll.kernel32.GetLastError() + raise RuntimeError( + f"SetProcessWorkingSetSizeEx failed with error code: {error_code}" + ) + return else: try: ctypes.CDLL("libc.so.6").malloc_trim(0) From 2bf1cb75152f0559617731faf67b88c889777d0a Mon Sep 17 00:00:00 2001 From: Vizonex Date: Sun, 27 Jul 2025 20:08:41 -0500 Subject: [PATCH 06/25] typo fix --- CHANGES/1233.contrib.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES/1233.contrib.rst b/CHANGES/1233.contrib.rst index 1ac7cce7d..ff5f12226 100644 --- a/CHANGES/1233.contrib.rst +++ b/CHANGES/1233.contrib.rst @@ -1,2 +1,2 @@ -Added memory leak test for popping or deleting attributes from a multidict to prevent future issues or bougus claims. +Added memory leak test for popping or deleting attributes from a multidict to prevent future issues or bogus claims. -- by :user:`Vizonex` From 45f4066c56f34965707305c754918b9315b620be Mon Sep 17 00:00:00 2001 From: Vizonex Date: Wed, 30 Jul 2025 10:27:27 -0500 Subject: [PATCH 07/25] see if this works... --- tests/isolated/multidict_pop.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 7a269b7ad..38c4b2888 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -55,9 +55,9 @@ def trim_ram() -> None: return else: try: - ctypes.CDLL("libc.so.6").malloc_trim(0) + ctypes.CDLL("libc.so").malloc_trim(0) except Exception as e: - print(" attempt failed") + print("FAILED: ", e) raise e From 7843457af78c0a850b154f1965921794c71679bd Mon Sep 17 00:00:00 2001 From: Vizonex Date: Wed, 30 Jul 2025 10:38:55 -0500 Subject: [PATCH 08/25] revert --- tests/isolated/multidict_pop.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 38c4b2888..5f15a0225 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -55,7 +55,7 @@ def trim_ram() -> None: return else: try: - ctypes.CDLL("libc.so").malloc_trim(0) + ctypes.CDLL("libc.so.6").malloc_trim(0) except Exception as e: print("FAILED: ", e) raise e From 9712cfee092f8cc596af98412f0acf45b1e896dc Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 14:46:11 -0500 Subject: [PATCH 09/25] Fixed Memory Leak with deleting and adding values --- multidict/_multidict.c | 5 ++- multidict/_multilib/hashtable.h | 70 ++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/multidict/_multidict.c b/multidict/_multidict.c index 73c002296..3d3e0e245 100644 --- a/multidict/_multidict.c +++ b/multidict/_multidict.c @@ -674,7 +674,6 @@ multidict_popone(MultiDictObject *self, PyObject *const *args, if (md_pop_one(self, key, &ret_val) < 0) { return NULL; } - ASSERT_CONSISTENT(self, false); if (ret_val == NULL) { if (_default != NULL) { @@ -720,6 +719,8 @@ multidict_pop(MultiDictObject *self, PyObject *const *args, Py_ssize_t nargs, return NULL; } } else { + // Py_DECREF(key); + // printf("%zu\n", ret_val->ob_refcnt); return ret_val; } } @@ -755,6 +756,8 @@ multidict_popall(MultiDictObject *self, PyObject *const *args, return NULL; } } else { + // printf("%zu", key->ob_refcnt); + // printf("%zu", ret_val->ob_refcnt); return ret_val; } } diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index 16b82ded1..c8300ac7f 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -91,6 +91,8 @@ static inline int _md_check_consistency(MultiDictObject *md, bool update); static inline int _md_dump(MultiDictObject *md); +static inline int +_md_refdump(MultiDictObject *md); #define ASSERT_CONSISTENT(md, update) assert(_md_check_consistency(md, update)) #else @@ -102,12 +104,12 @@ _str_cmp(PyObject *s1, PyObject *s2) { PyObject *ret = PyUnicode_RichCompare(s1, s2, Py_EQ); if (Py_IsTrue(ret)) { - Py_DECREF(ret); + Py_XDECREF(ret); return 1; } else if (ret == NULL) { return -1; } else { - Py_DECREF(ret); + Py_XDECREF(ret); return 0; } } @@ -481,7 +483,7 @@ static inline int _md_add_for_upd(MultiDictObject *md, Py_hash_t hash, PyObject *identity, PyObject *key, PyObject *value) { - Py_INCREF(identity); + // Py_INCREF(identity); Py_INCREF(key); Py_INCREF(value); return _md_add_for_upd_steal_refs(md, hash, identity, key, value); @@ -500,6 +502,7 @@ md_add(MultiDictObject *md, PyObject *key, PyObject *value) } int ret = _md_add_with_hash(md, hash, identity, key, value); ASSERT_CONSISTENT(md, false); + Py_DECREF(key); Py_DECREF(identity); return ret; fail: @@ -1010,16 +1013,15 @@ md_pop_one(MultiDictObject *md, PyObject *key, PyObject **ret) if (_md_del_at(md, iter.slot, entry) < 0) { goto fail; } - Py_DECREF(identity); *ret = value; md->version = NEXT_VERSION(md->state); - ASSERT_CONSISTENT(md, false); + Py_DECREF(identity); return 1; } else if (tmp < 0) { goto fail; } } - + Py_DECREF(identity); ASSERT_CONSISTENT(md, false); return 0; fail: @@ -1084,7 +1086,7 @@ md_pop_all(MultiDictObject *md, PyObject *key, PyObject **ret) } *ret = lst; - Py_DECREF(identity); + Py_XDECREF(identity); ASSERT_CONSISTENT(md, false); return lst != NULL; fail: @@ -1877,19 +1879,19 @@ md_traverse(MultiDictObject *md, visitproc visit, void *arg) static inline int md_clear(MultiDictObject *md) { - if (md->used == 0) { - return 0; - } + // Remove This because sometimes tweaked multidicts or memory leaks speap + // through the cracks. if (md->used == 0) { + // return 0; + // } md->version = NEXT_VERSION(md->state); entry_t *entries = htkeys_entries(md->keys); for (Py_ssize_t pos = 0; pos < md->keys->nentries; pos++) { entry_t *entry = entries + pos; - if (entry->identity != NULL) { - Py_CLEAR(entry->identity); - Py_CLEAR(entry->key); - Py_CLEAR(entry->value); - } + // Py_CLEAR has null checks of it's own making it easier to free. + Py_CLEAR(entry->identity); + Py_CLEAR(entry->key); + Py_CLEAR(entry->value); } md->used = 0; @@ -1995,6 +1997,44 @@ _md_dump(MultiDictObject *md) printf("\n"); return 1; } + +static inline int +_md_refdump(MultiDictObject *md) +{ + htkeys_t *keys = md->keys; + printf("Refcounts Dump %p [%zd from %zd usable %zd nentries %zd]\n", + (void *)md, + md->used, + htkeys_nslots(keys), + keys->usable, + keys->nentries); + for (Py_ssize_t i = 0; i < htkeys_nslots(keys); i++) { + Py_ssize_t ix = htkeys_get_index(keys, i); + printf(" %zd -> %zd\n", i, ix); + } + printf(" --------\n"); + entry_t *entries = htkeys_entries(keys); + for (Py_ssize_t i = 0; i < keys->nentries; i++) { + entry_t *entry = &entries[i]; + PyObject *identity = entry->identity; + PyObject *key = entry->key; + PyObject *value = entry->value; + if (identity == NULL) { + printf(" %zd [should be deleted]", i); + } else { + printf(" %zd h=%20zd", i, entry->hash); + } + if (key != NULL) { + printf(", k=%zd", key->ob_refcnt); + } + if (value != NULL) { + printf(", v=%zd", value->ob_refcnt); + } + } + printf("\n"); + return 1; +} + #endif // NDEBUG #ifdef __cplusplus From 9fa0bfc9891ec98aca4670c24ae8cc74655cb80a Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 16:41:53 -0500 Subject: [PATCH 10/25] fix crashing and ensure multidicts can actually clear entries --- multidict/_multidict.c | 4 ---- multidict/_multilib/hashtable.h | 27 +++++++++++++-------------- 2 files changed, 13 insertions(+), 18 deletions(-) diff --git a/multidict/_multidict.c b/multidict/_multidict.c index 3d3e0e245..6951a3d04 100644 --- a/multidict/_multidict.c +++ b/multidict/_multidict.c @@ -719,8 +719,6 @@ multidict_pop(MultiDictObject *self, PyObject *const *args, Py_ssize_t nargs, return NULL; } } else { - // Py_DECREF(key); - // printf("%zu\n", ret_val->ob_refcnt); return ret_val; } } @@ -756,8 +754,6 @@ multidict_popall(MultiDictObject *self, PyObject *const *args, return NULL; } } else { - // printf("%zu", key->ob_refcnt); - // printf("%zu", ret_val->ob_refcnt); return ret_val; } } diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index c8300ac7f..81977b1df 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -483,7 +483,7 @@ static inline int _md_add_for_upd(MultiDictObject *md, Py_hash_t hash, PyObject *identity, PyObject *key, PyObject *value) { - // Py_INCREF(identity); + Py_INCREF(identity); Py_INCREF(key); Py_INCREF(value); return _md_add_for_upd_steal_refs(md, hash, identity, key, value); @@ -502,7 +502,6 @@ md_add(MultiDictObject *md, PyObject *key, PyObject *value) } int ret = _md_add_with_hash(md, hash, identity, key, value); ASSERT_CONSISTENT(md, false); - Py_DECREF(key); Py_DECREF(identity); return ret; fail: @@ -1021,7 +1020,7 @@ md_pop_one(MultiDictObject *md, PyObject *key, PyObject **ret) goto fail; } } - Py_DECREF(identity); + Py_XDECREF(identity); ASSERT_CONSISTENT(md, false); return 0; fail: @@ -1879,21 +1878,23 @@ md_traverse(MultiDictObject *md, visitproc visit, void *arg) static inline int md_clear(MultiDictObject *md) { - // Remove This because sometimes tweaked multidicts or memory leaks speap - // through the cracks. if (md->used == 0) { - // return 0; - // } + if (( + // (md->used == 0) && + (md->keys == &empty_htkeys)) || + (md->state == NULL)) { + return 0; + } md->version = NEXT_VERSION(md->state); - entry_t *entries = htkeys_entries(md->keys); for (Py_ssize_t pos = 0; pos < md->keys->nentries; pos++) { entry_t *entry = entries + pos; // Py_CLEAR has null checks of it's own making it easier to free. - Py_CLEAR(entry->identity); - Py_CLEAR(entry->key); - Py_CLEAR(entry->value); + if (entry->identity != NULL) { + Py_CLEAR(entry->identity); + Py_CLEAR(entry->key); + Py_CLEAR(entry->value); + } } - md->used = 0; if (md->keys != &empty_htkeys) { htkeys_free(md->keys); @@ -1912,7 +1913,6 @@ _md_check_consistency(MultiDictObject *md, bool update) #define CHECK(expr) assert(expr) // do { if (!(expr)) { assert(0 && Py_STRINGIFY(expr)); } } while (0) - htkeys_t *keys = md->keys; CHECK(keys != NULL); Py_ssize_t calc_usable = USABLE_FRACTION(htkeys_nslots(keys)); @@ -1931,7 +1931,6 @@ _md_check_consistency(MultiDictObject *md, bool update) Py_ssize_t ix = htkeys_get_index(keys, i); CHECK(DKIX_DUMMY <= ix && ix <= calc_usable); } - entry_t *entries = htkeys_entries(keys); for (Py_ssize_t i = 0; i < calc_usable; i++) { entry_t *entry = &entries[i]; From 1059ec86c5e6541b77a323678c853c04b3d4631c Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 17:01:41 -0500 Subject: [PATCH 11/25] remove malloc-trim there is not need for it here --- tests/isolated/multidict_pop.py | 51 ++------------------------------- 1 file changed, 3 insertions(+), 48 deletions(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 5f15a0225..8765ccd0d 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -1,4 +1,5 @@ -# Test for memory leaks surrounding deletion of values. +# Test for memory leaks surrounding deletion of values or +# bad cleanups. # SEE: https://github.com/aio-libs/multidict/issues/1232 # We want to make sure that bad predictions or bougus claims # of memory leaks can be prevented in the future. @@ -7,58 +8,12 @@ import sys import psutil import os -import ctypes from multidict import MultiDict -# Code was borrowed for testing with windows and other operating systems respectively. -# SEE: https://stackoverflow.com/a/79150440 - - def trim_ram() -> None: - """Forces python garbage collection. - Most importantly, calls malloc_trim/SetProcessWorkingSetSizeEx, which fixes pandas/libc (?) memory leak.""" - + """Forces python garbage collection.""" gc.collect() - if sys.platform == "win32": - SIZE_T = ( - ctypes.c_uint32 if ctypes.sizeof(ctypes.c_void_p) == 4 else ctypes.c_uint64 - ) - - # Get a handle to the current process - - pid = ctypes.windll.kernel32.GetCurrentProcess() - - # Sometimes FileNotFoundError can appear so the code was - # changed to handle that workaround. - - ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.argtypes = [ - ctypes.c_void_p, # Process handle - SIZE_T, # Minimum working set size - SIZE_T, # Maximum working set size - ctypes.c_ulong, # Flags - ] - ctypes.windll.kernel32.SetProcessWorkingSetSizeEx.restype = ctypes.c_bool - - # Define constants for SetProcessWorkingSetSizeEx - QUOTA_LIMITS_HARDWS_MIN_DISABLE = 0x00000002 - - # Attempt to set the working set size - if not ctypes.windll.kernel32.SetProcessWorkingSetSizeEx( - pid, SIZE_T(-1), SIZE_T(-1), QUOTA_LIMITS_HARDWS_MIN_DISABLE - ): - # Retrieve the error code - error_code = ctypes.windll.kernel32.GetLastError() - raise RuntimeError( - f"SetProcessWorkingSetSizeEx failed with error code: {error_code}" - ) - return - else: - try: - ctypes.CDLL("libc.so.6").malloc_trim(0) - except Exception as e: - print("FAILED: ", e) - raise e def get_memory_usage() -> int: From 9f2903c038c83407c7974ab6e0cf15a66c922d81 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 17:16:44 -0500 Subject: [PATCH 12/25] _md_refdump is not version compatable so I'll comment it out for now... --- multidict/_multilib/hashtable.h | 76 ++++++++++++++++----------------- 1 file changed, 38 insertions(+), 38 deletions(-) diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index 81977b1df..1b66f9fed 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -91,8 +91,8 @@ static inline int _md_check_consistency(MultiDictObject *md, bool update); static inline int _md_dump(MultiDictObject *md); -static inline int -_md_refdump(MultiDictObject *md); +// static inline int +// _md_refdump(MultiDictObject *md); #define ASSERT_CONSISTENT(md, update) assert(_md_check_consistency(md, update)) #else @@ -1997,42 +1997,42 @@ _md_dump(MultiDictObject *md) return 1; } -static inline int -_md_refdump(MultiDictObject *md) -{ - htkeys_t *keys = md->keys; - printf("Refcounts Dump %p [%zd from %zd usable %zd nentries %zd]\n", - (void *)md, - md->used, - htkeys_nslots(keys), - keys->usable, - keys->nentries); - for (Py_ssize_t i = 0; i < htkeys_nslots(keys); i++) { - Py_ssize_t ix = htkeys_get_index(keys, i); - printf(" %zd -> %zd\n", i, ix); - } - printf(" --------\n"); - entry_t *entries = htkeys_entries(keys); - for (Py_ssize_t i = 0; i < keys->nentries; i++) { - entry_t *entry = &entries[i]; - PyObject *identity = entry->identity; - PyObject *key = entry->key; - PyObject *value = entry->value; - if (identity == NULL) { - printf(" %zd [should be deleted]", i); - } else { - printf(" %zd h=%20zd", i, entry->hash); - } - if (key != NULL) { - printf(", k=%zd", key->ob_refcnt); - } - if (value != NULL) { - printf(", v=%zd", value->ob_refcnt); - } - } - printf("\n"); - return 1; -} +// static inline int +// _md_refdump(MultiDictObject *md) +// { +// htkeys_t *keys = md->keys; +// printf("Refcounts Dump %p [%zd from %zd usable %zd nentries %zd]\n", +// (void *)md, +// md->used, +// htkeys_nslots(keys), +// keys->usable, +// keys->nentries); +// for (Py_ssize_t i = 0; i < htkeys_nslots(keys); i++) { +// Py_ssize_t ix = htkeys_get_index(keys, i); +// printf(" %zd -> %zd\n", i, ix); +// } +// printf(" --------\n"); +// entry_t *entries = htkeys_entries(keys); +// for (Py_ssize_t i = 0; i < keys->nentries; i++) { +// entry_t *entry = &entries[i]; +// PyObject *identity = entry->identity; +// PyObject *key = entry->key; +// PyObject *value = entry->value; +// if (identity == NULL) { +// printf(" %zd [should be deleted]", i); +// } else { +// printf(" %zd h=%20zd", i, entry->hash); +// } +// if (key != NULL) { +// printf(", k=%zd", key->ob_refcnt); +// } +// if (value != NULL) { +// printf(", v=%zd", value->ob_refcnt); +// } +// } +// printf("\n"); +// return 1; +// } #endif // NDEBUG From ecb03e750b2a9d306ba67d4267a9e35fec5275ae Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 17:21:29 -0500 Subject: [PATCH 13/25] increase timeout so that workflow doesn't cut off --- .github/workflows/ci-cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index 2457650df..fb562faff 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -203,7 +203,7 @@ jobs: debug: '' fail-fast: false runs-on: ${{ matrix.os }}-latest - timeout-minutes: 15 + timeout-minutes: 20 continue-on-error: >- ${{ From fac44426e599b2e0fd799d7a29c6e998ed794f84 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 17:39:25 -0500 Subject: [PATCH 14/25] increase fault handler timeout so that debug versions can finish in time --- pytest.ini | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pytest.ini b/pytest.ini index f5c094b96..d2e0ed8b6 100644 --- a/pytest.ini +++ b/pytest.ini @@ -41,7 +41,7 @@ doctest_optionflags = ALLOW_UNICODE ELLIPSIS # Marks tests with an empty parameterset as xfail(run=False) empty_parameter_set_mark = xfail -faulthandler_timeout = 60 +faulthandler_timeout = 90 filterwarnings = error From bd37797dff7813ce4417ebca817ff536de437074 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 17:40:35 -0500 Subject: [PATCH 15/25] revert this was the worng timeout --- .github/workflows/ci-cd.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-cd.yml b/.github/workflows/ci-cd.yml index fb562faff..2457650df 100644 --- a/.github/workflows/ci-cd.yml +++ b/.github/workflows/ci-cd.yml @@ -203,7 +203,7 @@ jobs: debug: '' fail-fast: false runs-on: ${{ matrix.os }}-latest - timeout-minutes: 20 + timeout-minutes: 15 continue-on-error: >- ${{ From 4e67d1307e93ca82bdb795c91d898a9b387c1fdf Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 18:19:33 -0500 Subject: [PATCH 16/25] lessen amount of time memory leak takes to test --- tests/isolated/multidict_pop.py | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 8765ccd0d..a343860cd 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -16,27 +16,29 @@ def trim_ram() -> None: gc.collect() +process = psutil.Process(os.getpid()) + + def get_memory_usage() -> int: - process = psutil.Process(os.getpid()) memory_info = process.memory_info() return memory_info.rss / (1024 * 1024) # type: ignore[no-any-return] -keys = [f"X-Any-{i}" for i in range(1000)] +keys = [f"X-Any-{i}" for i in range(100)] headers = {key: key * 2 for key in keys} def check_for_leak() -> None: trim_ram() usage = get_memory_usage() - # We should never go over 100MB - if usage > 100: + # We should never go over 40MB + if usage > 40: sys.exit(1) def _test_pop() -> None: for _ in range(10): - for _ in range(1000): + for _ in range(100): result = MultiDict(headers) for k in keys: result.pop(k) @@ -45,7 +47,7 @@ def _test_pop() -> None: def _test_popall() -> None: for _ in range(10): - for _ in range(1000): + for _ in range(100): result = MultiDict(headers) for k in keys: result.popall(k) @@ -54,7 +56,7 @@ def _test_popall() -> None: def _test_popone() -> None: for _ in range(10): - for _ in range(1000): + for _ in range(100): result = MultiDict(headers) for k in keys: result.popone(k) @@ -63,7 +65,7 @@ def _test_popone() -> None: def _test_del() -> None: for _ in range(10): - for _ in range(1000): + for _ in range(100): result = MultiDict(headers) for k in keys: del result[k] @@ -75,7 +77,6 @@ def _run_isolated_case() -> None: _test_popall() _test_popone() _test_del() - sys.exit(0) if __name__ == "__main__": From c5001731e69d1bb324d2d7d6e65d63b6659ab68a Mon Sep 17 00:00:00 2001 From: Vizonex Date: Thu, 7 Aug 2025 18:35:22 -0500 Subject: [PATCH 17/25] higher amount but print how big it was so we know where it should be capped --- tests/isolated/multidict_pop.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index a343860cd..6105f833a 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -31,8 +31,8 @@ def get_memory_usage() -> int: def check_for_leak() -> None: trim_ram() usage = get_memory_usage() - # We should never go over 40MB - if usage > 40: + if usage > 50: + print(f"Memory leaked at: {usage} MB") sys.exit(1) From ad1be492ab485aced04563e3f895fc462cb1f263 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Fri, 8 Aug 2025 11:57:19 -0500 Subject: [PATCH 18/25] make sure code-coverage does its job and fix up md_clear so that all values are being cleared --- multidict/_multilib/hashtable.h | 13 ++++--------- tests/isolated/multidict_pop.py | 5 +---- 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index 1b66f9fed..24d2d665a 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -1878,10 +1878,7 @@ md_traverse(MultiDictObject *md, visitproc visit, void *arg) static inline int md_clear(MultiDictObject *md) { - if (( - // (md->used == 0) && - (md->keys == &empty_htkeys)) || - (md->state == NULL)) { + if ((md->keys == &empty_htkeys) || (md->state == NULL)) { return 0; } md->version = NEXT_VERSION(md->state); @@ -1889,11 +1886,9 @@ md_clear(MultiDictObject *md) for (Py_ssize_t pos = 0; pos < md->keys->nentries; pos++) { entry_t *entry = entries + pos; // Py_CLEAR has null checks of it's own making it easier to free. - if (entry->identity != NULL) { - Py_CLEAR(entry->identity); - Py_CLEAR(entry->key); - Py_CLEAR(entry->value); - } + Py_CLEAR(entry->identity); + Py_CLEAR(entry->key); + Py_CLEAR(entry->value); } md->used = 0; if (md->keys != &empty_htkeys) { diff --git a/tests/isolated/multidict_pop.py b/tests/isolated/multidict_pop.py index 6105f833a..daced359e 100644 --- a/tests/isolated/multidict_pop.py +++ b/tests/isolated/multidict_pop.py @@ -5,7 +5,6 @@ # of memory leaks can be prevented in the future. import gc -import sys import psutil import os from multidict import MultiDict @@ -31,9 +30,7 @@ def get_memory_usage() -> int: def check_for_leak() -> None: trim_ram() usage = get_memory_usage() - if usage > 50: - print(f"Memory leaked at: {usage} MB") - sys.exit(1) + assert usage < 50, f"Memory leaked at: {usage} MB" def _test_pop() -> None: From e3743298694022aaca8dc89852f83732d29e0c2d Mon Sep 17 00:00:00 2001 From: Vizonex Date: Fri, 8 Aug 2025 19:59:09 -0500 Subject: [PATCH 19/25] label that we bugfixed a problem --- CHANGES/1233.bugfix.rst | 2 ++ 1 file changed, 2 insertions(+) create mode 100644 CHANGES/1233.bugfix.rst diff --git a/CHANGES/1233.bugfix.rst b/CHANGES/1233.bugfix.rst new file mode 100644 index 000000000..e5a659027 --- /dev/null +++ b/CHANGES/1233.bugfix.rst @@ -0,0 +1,2 @@ +Fix ``MutliDict`` & ``CIMultiDict`` memory leak when deleting values or clearing them +-- by :user:`Vizonex` From e1c5f14c5778b0fead33198da414a92d20a9de46 Mon Sep 17 00:00:00 2001 From: Vizonex Date: Sun, 10 Aug 2025 16:02:54 -0500 Subject: [PATCH 20/25] cleanup debug artifacts --- multidict/_multilib/hashtable.h | 50 ++++----------------------------- 1 file changed, 6 insertions(+), 44 deletions(-) diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index 24d2d665a..71711fb29 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -91,8 +91,6 @@ static inline int _md_check_consistency(MultiDictObject *md, bool update); static inline int _md_dump(MultiDictObject *md); -// static inline int -// _md_refdump(MultiDictObject *md); #define ASSERT_CONSISTENT(md, update) assert(_md_check_consistency(md, update)) #else @@ -104,12 +102,12 @@ _str_cmp(PyObject *s1, PyObject *s2) { PyObject *ret = PyUnicode_RichCompare(s1, s2, Py_EQ); if (Py_IsTrue(ret)) { - Py_XDECREF(ret); + Py_DECREF(ret); return 1; } else if (ret == NULL) { return -1; } else { - Py_XDECREF(ret); + Py_DECREF(ret); return 0; } } @@ -1012,15 +1010,16 @@ md_pop_one(MultiDictObject *md, PyObject *key, PyObject **ret) if (_md_del_at(md, iter.slot, entry) < 0) { goto fail; } + Py_DECREF(identity); *ret = value; md->version = NEXT_VERSION(md->state); - Py_DECREF(identity); + ASSERT_CONSISTENT(md, false); return 1; } else if (tmp < 0) { goto fail; } } - Py_XDECREF(identity); + ASSERT_CONSISTENT(md, false); return 0; fail: @@ -1085,7 +1084,7 @@ md_pop_all(MultiDictObject *md, PyObject *key, PyObject **ret) } *ret = lst; - Py_XDECREF(identity); + Py_DECREF(identity); ASSERT_CONSISTENT(md, false); return lst != NULL; fail: @@ -1992,43 +1991,6 @@ _md_dump(MultiDictObject *md) return 1; } -// static inline int -// _md_refdump(MultiDictObject *md) -// { -// htkeys_t *keys = md->keys; -// printf("Refcounts Dump %p [%zd from %zd usable %zd nentries %zd]\n", -// (void *)md, -// md->used, -// htkeys_nslots(keys), -// keys->usable, -// keys->nentries); -// for (Py_ssize_t i = 0; i < htkeys_nslots(keys); i++) { -// Py_ssize_t ix = htkeys_get_index(keys, i); -// printf(" %zd -> %zd\n", i, ix); -// } -// printf(" --------\n"); -// entry_t *entries = htkeys_entries(keys); -// for (Py_ssize_t i = 0; i < keys->nentries; i++) { -// entry_t *entry = &entries[i]; -// PyObject *identity = entry->identity; -// PyObject *key = entry->key; -// PyObject *value = entry->value; -// if (identity == NULL) { -// printf(" %zd [should be deleted]", i); -// } else { -// printf(" %zd h=%20zd", i, entry->hash); -// } -// if (key != NULL) { -// printf(", k=%zd", key->ob_refcnt); -// } -// if (value != NULL) { -// printf(", v=%zd", value->ob_refcnt); -// } -// } -// printf("\n"); -// return 1; -// } - #endif // NDEBUG #ifdef __cplusplus From fb25e169fc973c215ab984418677c1519e161669 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 11 Aug 2025 10:55:26 +0200 Subject: [PATCH 21/25] Apply suggestions from code review --- multidict/_multidict.c | 1 + multidict/_multilib/hashtable.h | 5 ++++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/multidict/_multidict.c b/multidict/_multidict.c index 6951a3d04..69b68328f 100644 --- a/multidict/_multidict.c +++ b/multidict/_multidict.c @@ -674,6 +674,7 @@ multidict_popone(MultiDictObject *self, PyObject *const *args, if (md_pop_one(self, key, &ret_val) < 0) { return NULL; } + ASSERT_CONSISTENT(self, false); if (ret_val == NULL) { if (_default != NULL) { diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index 71711fb29..e6847c73d 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -1881,6 +1881,7 @@ md_clear(MultiDictObject *md) return 0; } md->version = NEXT_VERSION(md->state); + entry_t *entries = htkeys_entries(md->keys); for (Py_ssize_t pos = 0; pos < md->keys->nentries; pos++) { entry_t *entry = entries + pos; @@ -1889,6 +1890,7 @@ md_clear(MultiDictObject *md) Py_CLEAR(entry->key); Py_CLEAR(entry->value); } + md->used = 0; if (md->keys != &empty_htkeys) { htkeys_free(md->keys); @@ -1907,6 +1909,7 @@ _md_check_consistency(MultiDictObject *md, bool update) #define CHECK(expr) assert(expr) // do { if (!(expr)) { assert(0 && Py_STRINGIFY(expr)); } } while (0) + htkeys_t *keys = md->keys; CHECK(keys != NULL); Py_ssize_t calc_usable = USABLE_FRACTION(htkeys_nslots(keys)); @@ -1925,6 +1928,7 @@ _md_check_consistency(MultiDictObject *md, bool update) Py_ssize_t ix = htkeys_get_index(keys, i); CHECK(DKIX_DUMMY <= ix && ix <= calc_usable); } + entry_t *entries = htkeys_entries(keys); for (Py_ssize_t i = 0; i < calc_usable; i++) { entry_t *entry = &entries[i]; @@ -1990,7 +1994,6 @@ _md_dump(MultiDictObject *md) printf("\n"); return 1; } - #endif // NDEBUG #ifdef __cplusplus From 0ccaf30e8fbfcb16a2c2aaafb0c9e464e86af9cf Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 11 Aug 2025 11:00:11 +0200 Subject: [PATCH 22/25] Apply suggestions from code review --- multidict/_multidict.c | 1 - multidict/_multilib/hashtable.h | 4 ---- 2 files changed, 5 deletions(-) diff --git a/multidict/_multidict.c b/multidict/_multidict.c index 69b68328f..6951a3d04 100644 --- a/multidict/_multidict.c +++ b/multidict/_multidict.c @@ -674,7 +674,6 @@ multidict_popone(MultiDictObject *self, PyObject *const *args, if (md_pop_one(self, key, &ret_val) < 0) { return NULL; } - ASSERT_CONSISTENT(self, false); if (ret_val == NULL) { if (_default != NULL) { diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index e6847c73d..4929e1144 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -1881,7 +1881,6 @@ md_clear(MultiDictObject *md) return 0; } md->version = NEXT_VERSION(md->state); - entry_t *entries = htkeys_entries(md->keys); for (Py_ssize_t pos = 0; pos < md->keys->nentries; pos++) { entry_t *entry = entries + pos; @@ -1890,7 +1889,6 @@ md_clear(MultiDictObject *md) Py_CLEAR(entry->key); Py_CLEAR(entry->value); } - md->used = 0; if (md->keys != &empty_htkeys) { htkeys_free(md->keys); @@ -1909,7 +1907,6 @@ _md_check_consistency(MultiDictObject *md, bool update) #define CHECK(expr) assert(expr) // do { if (!(expr)) { assert(0 && Py_STRINGIFY(expr)); } } while (0) - htkeys_t *keys = md->keys; CHECK(keys != NULL); Py_ssize_t calc_usable = USABLE_FRACTION(htkeys_nslots(keys)); @@ -1928,7 +1925,6 @@ _md_check_consistency(MultiDictObject *md, bool update) Py_ssize_t ix = htkeys_get_index(keys, i); CHECK(DKIX_DUMMY <= ix && ix <= calc_usable); } - entry_t *entries = htkeys_entries(keys); for (Py_ssize_t i = 0; i < calc_usable; i++) { entry_t *entry = &entries[i]; From 232806bc4c61279245cc8473c04c924315204294 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 11 Aug 2025 11:10:59 +0200 Subject: [PATCH 23/25] Apply suggestions from code review --- multidict/_multidict.c | 1 + multidict/_multilib/hashtable.h | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/multidict/_multidict.c b/multidict/_multidict.c index 6951a3d04..73c002296 100644 --- a/multidict/_multidict.c +++ b/multidict/_multidict.c @@ -674,6 +674,7 @@ multidict_popone(MultiDictObject *self, PyObject *const *args, if (md_pop_one(self, key, &ret_val) < 0) { return NULL; } + ASSERT_CONSISTENT(self, false); if (ret_val == NULL) { if (_default != NULL) { diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index 4929e1144..c2f686b4c 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -1881,14 +1881,17 @@ md_clear(MultiDictObject *md) return 0; } md->version = NEXT_VERSION(md->state); + entry_t *entries = htkeys_entries(md->keys); for (Py_ssize_t pos = 0; pos < md->keys->nentries; pos++) { entry_t *entry = entries + pos; - // Py_CLEAR has null checks of it's own making it easier to free. - Py_CLEAR(entry->identity); - Py_CLEAR(entry->key); - Py_CLEAR(entry->value); + if (entry->identity != NULL) { + Py_CLEAR(entry->identity); + Py_CLEAR(entry->key); + Py_CLEAR(entry->value); + } } + md->used = 0; if (md->keys != &empty_htkeys) { htkeys_free(md->keys); @@ -1907,6 +1910,7 @@ _md_check_consistency(MultiDictObject *md, bool update) #define CHECK(expr) assert(expr) // do { if (!(expr)) { assert(0 && Py_STRINGIFY(expr)); } } while (0) + htkeys_t *keys = md->keys; CHECK(keys != NULL); Py_ssize_t calc_usable = USABLE_FRACTION(htkeys_nslots(keys)); @@ -1925,6 +1929,7 @@ _md_check_consistency(MultiDictObject *md, bool update) Py_ssize_t ix = htkeys_get_index(keys, i); CHECK(DKIX_DUMMY <= ix && ix <= calc_usable); } + entry_t *entries = htkeys_entries(keys); for (Py_ssize_t i = 0; i < calc_usable; i++) { entry_t *entry = &entries[i]; From 3a08bfc8b30e49025f0caa358699a84e1d012579 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 11 Aug 2025 11:14:31 +0200 Subject: [PATCH 24/25] Update multidict/_multilib/hashtable.h --- multidict/_multilib/hashtable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index c2f686b4c..e5c878ac9 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -1877,7 +1877,7 @@ md_traverse(MultiDictObject *md, visitproc visit, void *arg) static inline int md_clear(MultiDictObject *md) { - if ((md->keys == &empty_htkeys) || (md->state == NULL)) { + if (md->keys == &empty_htkeys) { return 0; } md->version = NEXT_VERSION(md->state); From 53b7bb62cb2a8605ba4a3ad4f0ea774244996377 Mon Sep 17 00:00:00 2001 From: Andrew Svetlov Date: Mon, 11 Aug 2025 11:37:56 +0200 Subject: [PATCH 25/25] Update multidict/_multilib/hashtable.h --- multidict/_multilib/hashtable.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/multidict/_multilib/hashtable.h b/multidict/_multilib/hashtable.h index e5c878ac9..82019f9eb 100644 --- a/multidict/_multilib/hashtable.h +++ b/multidict/_multilib/hashtable.h @@ -1877,7 +1877,7 @@ md_traverse(MultiDictObject *md, visitproc visit, void *arg) static inline int md_clear(MultiDictObject *md) { - if (md->keys == &empty_htkeys) { + if (md->keys == NULL || md->keys == &empty_htkeys) { return 0; } md->version = NEXT_VERSION(md->state);