From 55a72a819417a74f096ecdf89729571cbfc62e2e Mon Sep 17 00:00:00 2001 From: Piotr Rozyczko Date: Sun, 8 Feb 2026 10:00:59 +0100 Subject: [PATCH] attempt at creating monotonic unique_id counters --- .../global_object/global_object.py | 32 +++++++++++------- src/easyscience/global_object/map.py | 4 +++ .../global_object/test_global_object.py | 33 ++++++++++++++++++- 3 files changed, 56 insertions(+), 13 deletions(-) diff --git a/src/easyscience/global_object/global_object.py b/src/easyscience/global_object/global_object.py index 63726d01..2e2c2ae6 100644 --- a/src/easyscience/global_object/global_object.py +++ b/src/easyscience/global_object/global_object.py @@ -46,20 +46,28 @@ def instantiate_stack(self): def generate_unique_name(self, name_prefix: str) -> str: """ - Generate a generic unique name for the object using the class name and a global iterator. + Generate a generic unique name for the object using the class name and a monotonic counter. Names are in the format `name_prefix_0`, `name_prefix_1`, `name_prefix_2`, etc. + The counter for each prefix only ever increases, ensuring that names are never + reused even after objects are garbage-collected from the map. + :param name_prefix: The prefix to be used for the name """ + # Get the stored counter for this prefix (-1 means no name generated yet) + current_counter = self.map._name_counters.get(name_prefix, -1) + + # Also check existing map entries to handle explicitly named objects + # (e.g. created with a user-supplied unique_name or deserialized) + max_from_map = -1 names_with_prefix = [name for name in self.map.vertices() if name.startswith(name_prefix + '_')] - if names_with_prefix: - name_with_prefix_count = [0] - for name in names_with_prefix: - # Strip away the prefix and trailing _ - name_without_prefix = name.replace(name_prefix + '_', '') - if name_without_prefix.isdecimal(): - name_with_prefix_count.append(int(name_without_prefix)) - unique_name = f'{name_prefix}_{max(name_with_prefix_count) + 1}' - else: - unique_name = f'{name_prefix}_0' - return unique_name + for name in names_with_prefix: + name_without_prefix = name.replace(name_prefix + '_', '') + if name_without_prefix.isdecimal(): + max_from_map = max(max_from_map, int(name_without_prefix)) + + # Take the maximum of counter and map, then increment + next_counter = max(current_counter, max_from_map) + 1 + self.map._name_counters[name_prefix] = next_counter + + return f'{name_prefix}_{next_counter}' diff --git a/src/easyscience/global_object/map.py b/src/easyscience/global_object/map.py index 7fb224b8..443e5e1a 100644 --- a/src/easyscience/global_object/map.py +++ b/src/easyscience/global_object/map.py @@ -73,6 +73,9 @@ def __init__(self): self._store = weakref.WeakValueDictionary() # A dict with object names as keys and a list of their object types as values, with weak references self.__type_dict = {} + # Per-prefix monotonic counters for unique name generation. + # These only ever increase, ensuring names are never reused after GC. + self._name_counters: dict = {} def vertices(self) -> List[str]: """Returns the vertices of a map. @@ -281,6 +284,7 @@ def _clear(self): """Reset the map to an empty state. Only to be used for testing""" self._store.clear() self.__type_dict.clear() + self._name_counters.clear() gc.collect() def __repr__(self) -> str: diff --git a/tests/unit_tests/global_object/test_global_object.py b/tests/unit_tests/global_object/test_global_object.py index 96cc4888..19c31d97 100644 --- a/tests/unit_tests/global_object/test_global_object.py +++ b/tests/unit_tests/global_object/test_global_object.py @@ -40,7 +40,7 @@ def test_generate_unique_name_already_taken(self): @pytest.fixture def clear_global_map(self): - """Clear global map before and after each test""" + """Clear global map and name counters before and after each test""" global_object.map._clear() yield global_object.map._clear() @@ -154,6 +154,37 @@ def test_integration_with_parameter_creation(self, clear_global_map): retrieved1 = global_obj.map.get_item_by_key(param1.unique_name) assert retrieved1 is param1 + def test_unique_names_not_reused_after_gc(self, clear_global_map): + """Test that unique names are never reused after objects are garbage collected.""" + import gc + + # Given + global_obj = GlobalObject() + + # Create parameters and record their unique names + param1 = Parameter(name="a", value=1.0) + param2 = Parameter(name="b", value=2.0) + name1 = param1.unique_name + name2 = param2.unique_name + assert name1 != name2 + + # Delete the parameters so they get GC'd from the WeakValueDictionary + del param1, param2 + gc.collect() + + # The map should no longer contain the old names (weak refs collected) + assert name1 not in global_obj.map.vertices() + assert name2 not in global_obj.map.vertices() + + # Create new parameters — they must NOT reuse the old names + param3 = Parameter(name="c", value=3.0) + param4 = Parameter(name="d", value=4.0) + assert param3.unique_name != name1 + assert param3.unique_name != name2 + assert param4.unique_name != name1 + assert param4.unique_name != name2 + assert param3.unique_name != param4.unique_name + def test_script_manager_access(self): """Test that script manager is accessible""" # Given