Merge pull request #113 from DavidCEllis/fix-potential-race

Fix potential race

Author: DavidCEllis

docs/code_examples/docs_ex10_frozen_attributes.py

@@ -94,8 +94,10 @@ def freezable(cls=None, /, *, frozen=False):
     # Frozen attributes need to be added afterwards
     # Due to the need to know if frozen fields exist
     if frozen:
-        setattr(cls, "__setattr__", dtmethods.frozen_setattr_maker)
-        setattr(cls, "__delattr__", dtmethods.frozen_delattr_maker)
+        dtmethods.add_methods(
+            cls,
+            [dtmethods.frozen_setattr_maker, dtmethods.frozen_delattr_maker]
+        )
     else:
         fields = dtfuncs.get_fields(cls)
         has_frozen_fields = False
@@ -105,8 +107,10 @@ def freezable(cls=None, /, *, frozen=False):
                 break
 
         if has_frozen_fields:
-            setattr(cls, "__setattr__", frozen_setattr_field_maker)
-            setattr(cls, "__delattr__", frozen_delattr_field_maker)
+            dtmethods.add_methods(
+                cls,
+                [frozen_setattr_field_maker, frozen_delattr_field_maker]
+            )
 
     return cls
 

src/ducktools/classbuilder/__init__.py

@@ -463,8 +463,8 @@ def __eq__(self, other):
     def __repr__(self):
         return f"{type(self).__name__}(fields={self.fields!r}, modifications={self.modifications!r})"
 
-    def __call__(self, cls_dict):
-        # cls_dict will be provided, but isn't needed
+    def __call__(self, cls_or_ns):
+        # cls_or_ns will be provided, but isn't needed
         return self.fields, self.modifications
 
 

src/ducktools/classbuilder/__init__.pyi

@@ -139,7 +139,7 @@ class GatheredFields:
     def __repr__(self) -> str: ...
     def __eq__(self, other) -> bool: ...
     def __call__(
-        self, cls_dict: type | dict[str, typing.Any]
+        self, cls_or_ns: type | dict[str, typing.Any]
     ) -> tuple[dict[str, Field], dict[str, typing.Any]]: ...
 
 # Only technically frozen under testing but we should *act* like they are frozen

src/ducktools/classbuilder/functions.py

@@ -47,7 +47,7 @@ def get_fields(cls, *, local=False):
     """
     key = "local_fields" if local else "fields"
     try:
-        return getattr(cls, INTERNALS_DICT)[key]
+        return cls.__dict__[INTERNALS_DICT][key]
     except (AttributeError, KeyError):
         raise TypeError(f"{cls} is not a classbuilder generated class")
 
@@ -61,7 +61,7 @@ def get_flags(cls):
     :return: dictionary of keys and flag values
     """
     try:
-        return getattr(cls, INTERNALS_DICT)["flags"]
+        return cls.__dict__[INTERNALS_DICT]["flags"]
     except (AttributeError, KeyError):
         raise TypeError(f"{cls} is not a classbuilder generated class")
 
@@ -75,7 +75,7 @@ def get_methods(cls):
     :return: dict of generated methods attached to the class by name
     """
     try:
-        return getattr(cls, INTERNALS_DICT)["methods"]
+        return cls.__dict__[INTERNALS_DICT]["methods"]
     except (AttributeError, KeyError):
         raise TypeError(f"{cls} is not a classbuilder generated class")
 

src/ducktools/classbuilder/methods.py

@@ -40,7 +40,7 @@
 
 from .annotations import apply_annotations
 from .constants import INTERNALS_DICT, NOTHING, REPLACE_NAME
-from .functions import get_fields, get_flags
+from .functions import get_fields, get_flags, get_methods
 
 try:
     from ._cached_methods import init_cache, setattr_cache
@@ -109,6 +109,15 @@ def generate(self):
         return method
 
 
+def _get_method(cls, name):
+    try:
+        methods = get_methods(cls)
+    except TypeError:
+        return None
+
+    return methods.get(name, None)
+
+
 class MethodMaker:
     """
     The descriptor class to place where methods should be generated.
@@ -135,16 +144,18 @@ def __repr__(self):
         return f"<MethodMaker for {self.funcname!r} method>"
 
     def __get__(self, inst, cls):
-        # This can be called via super().funcname(...) in which case the class
+        # This can be called via a subclass or through
+        # super().funcname(...) in which case the class
         # may not be the correct one. If this is the correct class
         # it should have this descriptor in the class dict under
         # the correct funcname.
         # Otherwise is should be found in the MRO of the class.
-        if cls.__dict__.get(self.funcname) is self:
+
+        if _get_method(cls, self.funcname) is self:
             gen_cls = cls
         else:
             for c in cls.__mro__[1:]:  # skip 'cls' as special cased
-                if c.__dict__.get(self.funcname) is self:
+                if _get_method(c, self.funcname) is self:
                     gen_cls = c
                     break
             else:

tests/test_core.py

@@ -4,31 +4,37 @@
 import pickle
 
 from ducktools.classbuilder import (
+    builder,
+    default_methods,
+    make_unified_gatherer,
+    slot_gatherer,
+    slotclass,
+
+    Field,
+    GatheredFields,
+    SlotFields,
+)
+from ducktools.classbuilder.constants import (
     _NothingType,
 
     INTERNALS_DICT,
     NOTHING,
     FIELD_NOTHING,
+)
+from ducktools.classbuilder.functions import (
+    get_fields,
+    get_flags,
+    get_methods,
+)
+from ducktools.classbuilder.methods import (
+    GeneratedCode,
+    MethodMaker,
 
     add_methods,
-    builder,
-    default_methods,
     eq_maker,
     frozen_delattr_maker,
     frozen_setattr_maker,
-    get_fields,
-    get_flags,
-    get_methods,
     init_maker,
-    make_unified_gatherer,
-    slot_gatherer,
-    slotclass,
-
-    Field,
-    GatheredFields,
-    GeneratedCode,
-    MethodMaker,
-    SlotFields,
 )
 from ducktools.classbuilder.annotations import get_ns_annotations
 
@@ -68,12 +74,14 @@ def generator(cls, funcname="demo"):
 
     assert repr(method_desc) == "<MethodMaker for 'demo' method>"
 
+    gatherer = GatheredFields({}, {})
+    @builder(gatherer=gatherer, methods={method_desc})
     class ValueX:
-        demo = method_desc
-
         def __init__(self):
             self.x = "Example Value"
 
+    add_methods(ValueX, [method_desc])
+
     ex = ValueX()
 
     assert ValueX.__dict__["demo"] == method_desc