From b9a05dcc3cafc02cfdb74e31a056a4ac8bcbf721 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 11 Dec 2024 19:30:16 +0100 Subject: [PATCH 1/9] add singledispatchmethod test for objects with equal hash and == --- Lib/test/test_functools.py | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index 32224866082824..a145d6b64c2d35 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -3240,6 +3240,25 @@ def f(arg): def _(arg: undefined): return "forward reference" + def test_singledispatchmethod_hash_comparision_equal(self): + from dataclasses import dataclass + + @dataclass(frozen=True) + class A: + value: int + + @functools.singledispatchmethod + def dispatch(self, x): + return id(self) + + t1 = A(1) + t2 = A(1) + + assert t1 == t2 + assert id(t1) == t1.dispatch(2) + assert id(t2) == t2.dispatch(2) # gh-127750 + + class CachedCostItem: _cost = 1 From 0714c06ac539a8dbdcb4d43b4bd41667857f03e9 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 11 Dec 2024 19:44:17 +0100 Subject: [PATCH 2/9] add singledispatchmethod test for dangling references --- Lib/test/test_functools.py | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/Lib/test/test_functools.py b/Lib/test/test_functools.py index a145d6b64c2d35..6d6ddac5e2f90b 100644 --- a/Lib/test/test_functools.py +++ b/Lib/test/test_functools.py @@ -3,6 +3,7 @@ import collections import collections.abc import copy +import gc from itertools import permutations import pickle from random import choice @@ -3258,6 +3259,33 @@ def dispatch(self, x): assert id(t1) == t1.dispatch(2) assert id(t2) == t2.dispatch(2) # gh-127750 + def test_singledispatchmethod_object_references(self): + class ReferenceTest: + instance_counter = 0 + + def __init__(self): + ReferenceTest.instance_counter = ReferenceTest.instance_counter + 1 + + def __del__(self): + ReferenceTest.instance_counter = ReferenceTest.instance_counter - 1 + + @functools.singledispatchmethod + def go(self, item): + pass + + assert ReferenceTest.instance_counter == 0 + t=ReferenceTest() + assert ReferenceTest.instance_counter == 1 + x = [] + for ii in range(1000): + t = ReferenceTest() + t.go(ii) + x.append(t) + del t + del x + gc.collect() + assert ReferenceTest.instance_counter == 0 + class CachedCostItem: _cost = 1 From ef03f2d9c0e8d3a79f972faaf25906ed2ac0ceaa Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 11 Dec 2024 19:45:50 +0100 Subject: [PATCH 3/9] use id for object cache in singledispatchmethod --- Lib/functools.py | 21 +++++++++------------ 1 file changed, 9 insertions(+), 12 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index fd33f0ae479ddc..dd37af083b5947 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1027,7 +1027,7 @@ def __init__(self, func): self.func = func import weakref # see comment in singledispatch function - self._method_cache = weakref.WeakKeyDictionary() + self._method_cache = weakref.WeakValueDictionary() def register(self, cls, method=None): """generic_method.register(cls, func) -> func @@ -1037,15 +1037,12 @@ def register(self, cls, method=None): return self.dispatcher.register(cls, func=method) def __get__(self, obj, cls=None): - if self._method_cache is not None: - try: - _method = self._method_cache[obj] - except TypeError: - self._method_cache = None - except KeyError: - pass - else: - return _method + try: + _method = self._method_cache[id(obj)] + except KeyError: + pass + else: + return _method dispatch = self.dispatcher.dispatch funcname = getattr(self.func, '__name__', 'singledispatchmethod method') @@ -1055,12 +1052,12 @@ def _method(*args, **kwargs): '1 positional argument') return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs) + _method.__self_reference = _method # create strong reference to _method. see gh-127751 _method.__isabstractmethod__ = self.__isabstractmethod__ _method.register = self.register update_wrapper(_method, self.func) - if self._method_cache is not None: - self._method_cache[obj] = _method + self._method_cache[id(obj)] = _method return _method From 6c96b9d8d3670518a17a3a653e5aa668de0539c4 Mon Sep 17 00:00:00 2001 From: "blurb-it[bot]" <43283697+blurb-it[bot]@users.noreply.github.com> Date: Wed, 11 Dec 2024 20:26:47 +0000 Subject: [PATCH 4/9] =?UTF-8?q?=F0=9F=93=9C=F0=9F=A4=96=20Added=20by=20blu?= =?UTF-8?q?rb=5Fit.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../next/Library/2024-12-11-20-26-44.gh-issue-127750.8witaH.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 Misc/NEWS.d/next/Library/2024-12-11-20-26-44.gh-issue-127750.8witaH.rst diff --git a/Misc/NEWS.d/next/Library/2024-12-11-20-26-44.gh-issue-127750.8witaH.rst b/Misc/NEWS.d/next/Library/2024-12-11-20-26-44.gh-issue-127750.8witaH.rst new file mode 100644 index 00000000000000..9ebf07e958e10d --- /dev/null +++ b/Misc/NEWS.d/next/Library/2024-12-11-20-26-44.gh-issue-127750.8witaH.rst @@ -0,0 +1 @@ +Update caching of :class:`functools.singledispatchmethod` to avoid collisions of objects which compare equal. From f282ab896ba5c4fc12050ca2279df60d0e28ebcd Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 8 Jan 2025 21:13:32 +0100 Subject: [PATCH 5/9] rework using __dict__ --- Lib/functools.py | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index dd37af083b5947..684524329161ae 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1029,6 +1029,9 @@ def __init__(self, func): import weakref # see comment in singledispatch function self._method_cache = weakref.WeakValueDictionary() + def __set_name__(self, obj, name): + self.attrname = name + def register(self, cls, method=None): """generic_method.register(cls, func) -> func @@ -1037,27 +1040,35 @@ def register(self, cls, method=None): return self.dispatcher.register(cls, func=method) def __get__(self, obj, cls=None): + try: - _method = self._method_cache[id(obj)] - except KeyError: + cache = obj.__dict__ + except AttributeError: + # how to disable caching for next invocation? ) + # 1) do not disable, but accept the AttributeError on each invocation or use hasattr. makes the operation (a bit) slower for slotted classes + # 2) remember in a WeakValueDictionary: self.no_cache with key id(obj) and value obj + # picking first option for now + cache = None pass else: - return _method + method = cache.get(self.attrname) + if method is not None: + return method dispatch = self.dispatcher.dispatch funcname = getattr(self.func, '__name__', 'singledispatchmethod method') + def _method(*args, **kwargs): if not args: raise TypeError(f'{funcname} requires at least ' '1 positional argument') return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs) - - _method.__self_reference = _method # create strong reference to _method. see gh-127751 _method.__isabstractmethod__ = self.__isabstractmethod__ _method.register = self.register update_wrapper(_method, self.func) - self._method_cache[id(obj)] = _method + if cache is not None: + cache[self.attrname] = _method return _method From 89a4a5c34d18062e05130a6904f5f5c58768a509 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 8 Jan 2025 21:21:32 +0100 Subject: [PATCH 6/9] remove comments --- Lib/functools.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index 684524329161ae..ff67bf810ef09f 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1026,9 +1026,6 @@ def __init__(self, func): self.dispatcher = singledispatch(func) self.func = func - import weakref # see comment in singledispatch function - self._method_cache = weakref.WeakValueDictionary() - def __set_name__(self, obj, name): self.attrname = name @@ -1040,16 +1037,10 @@ def register(self, cls, method=None): return self.dispatcher.register(cls, func=method) def __get__(self, obj, cls=None): - try: cache = obj.__dict__ except AttributeError: - # how to disable caching for next invocation? ) - # 1) do not disable, but accept the AttributeError on each invocation or use hasattr. makes the operation (a bit) slower for slotted classes - # 2) remember in a WeakValueDictionary: self.no_cache with key id(obj) and value obj - # picking first option for now cache = None - pass else: method = cache.get(self.attrname) if method is not None: From ecc3e823545a337a6afe416afea6395677ed4855 Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 8 Jan 2025 21:36:52 +0100 Subject: [PATCH 7/9] avoid exception --- Lib/functools.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/Lib/functools.py b/Lib/functools.py index ff67bf810ef09f..cccae955f5a31d 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1037,14 +1037,13 @@ def register(self, cls, method=None): return self.dispatcher.register(cls, func=method) def __get__(self, obj, cls=None): - try: + if hasattr(obj, '__dict__'): cache = obj.__dict__ - except AttributeError: - cache = None - else: method = cache.get(self.attrname) if method is not None: return method + else: + cache = None dispatch = self.dispatcher.dispatch funcname = getattr(self.func, '__name__', 'singledispatchmethod method') From efd4034748c846c7c781ea49fe3a4268b3ea26df Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 8 Jan 2025 21:49:33 +0100 Subject: [PATCH 8/9] whitespace --- Lib/functools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/functools.py b/Lib/functools.py index cccae955f5a31d..bfa1bd14408c75 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1047,12 +1047,12 @@ def __get__(self, obj, cls=None): dispatch = self.dispatcher.dispatch funcname = getattr(self.func, '__name__', 'singledispatchmethod method') - def _method(*args, **kwargs): if not args: raise TypeError(f'{funcname} requires at least ' '1 positional argument') return dispatch(args[0].__class__).__get__(obj, cls)(*args, **kwargs) + _method.__isabstractmethod__ = self.__isabstractmethod__ _method.register = self.register update_wrapper(_method, self.func) From 88b5e90d5f491afd07b1bf75c06a95f5f90744fa Mon Sep 17 00:00:00 2001 From: Pieter Eendebak Date: Wed, 8 Jan 2025 21:50:03 +0100 Subject: [PATCH 9/9] whitespace --- Lib/functools.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/functools.py b/Lib/functools.py index bfa1bd14408c75..a84e6a2472456f 100644 --- a/Lib/functools.py +++ b/Lib/functools.py @@ -1041,7 +1041,7 @@ def __get__(self, obj, cls=None): cache = obj.__dict__ method = cache.get(self.attrname) if method is not None: - return method + return method else: cache = None