Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 42 additions & 15 deletions src/easyscience/global_object/map.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,20 @@
# A dict with object names as keys and a list of their object types as values, with weak references
self.__type_dict = {}

def _snapshot_items(self):
"""Return a stable snapshot of __type_dict items.

Some callers iterate over __type_dict while other threads or
weakref finalizers may modify it. Creating a list snapshot (with
a retry loop) prevents RuntimeError: dictionary changed size during iteration.
"""
while True:
try:
return list(self.__type_dict.items())
except RuntimeError:

Check warning on line 87 in src/easyscience/global_object/map.py

View check run for this annotation

Codecov / codecov/patch

src/easyscience/global_object/map.py#L87

Added line #L87 was not covered by tests
# Dict changed during snapshot creation, retry
continue

Check warning on line 89 in src/easyscience/global_object/map.py

View check run for this annotation

Codecov / codecov/patch

src/easyscience/global_object/map.py#L89

Added line #L89 was not covered by tests

def vertices(self) -> List[str]:
"""Returns the vertices of a map.

Expand Down Expand Up @@ -109,7 +123,15 @@

def _nested_get(self, obj_type: str) -> List[str]:
"""Access a nested object in root by key sequence."""
return [key for key, item in self.__type_dict.items() if obj_type in item.type]
# Create a stable snapshot of the dict items to avoid RuntimeError
# when the dict is modified during iteration (e.g., by finalizers).
while True:
try:
items = self._snapshot_items()
return [key for key, item in items if obj_type in item.type]
except RuntimeError:

Check warning on line 132 in src/easyscience/global_object/map.py

View check run for this annotation

Codecov / codecov/patch

src/easyscience/global_object/map.py#L132

Added line #L132 was not covered by tests
# In case the snapshot itself raises (very rare), retry
continue

Check warning on line 134 in src/easyscience/global_object/map.py

View check run for this annotation

Codecov / codecov/patch

src/easyscience/global_object/map.py#L134

Added line #L134 was not covered by tests

def get_item_by_key(self, item_id: str) -> object:
if item_id in self._store:
Expand Down Expand Up @@ -143,10 +165,13 @@
# but the finalizer hasn't run yet
if name in self.__type_dict:
del self.__type_dict[name]

self._store[name] = obj
self.__type_dict[name] = _EntryList() # Add objects type to the list of types
self.__type_dict[name].finalizer = weakref.finalize(self._store[name], self.prune, name)
self.__type_dict[name].type = obj_type

entry_list = _EntryList()
entry_list.finalizer = weakref.finalize(obj, self.prune, name)
entry_list.type = obj_type
self.__type_dict[name] = entry_list # Add objects type to the list of types

def add_edge(self, start_obj: object, end_obj: object):
if start_obj.unique_name in self.__type_dict:
Expand All @@ -167,8 +192,11 @@
vertices
"""
edges = []
for vertex in self.__type_dict:
for neighbour in self.__type_dict[vertex]:
# Iterate over a snapshot of items and snapshot neighbour lists to
# avoid concurrent modification issues.
for vertex, neighbours in self._snapshot_items():
neighbours_snapshot = list(neighbours)
for neighbour in neighbours_snapshot:
if {neighbour, vertex} not in edges:
edges.append({vertex, neighbour})
return edges
Expand All @@ -190,12 +218,10 @@

def find_isolated_vertices(self) -> list:
"""returns a list of isolated vertices."""
graph = self.__type_dict
isolated = []
for vertex in graph:
print(isolated, vertex)
if not graph[vertex]:
isolated += [vertex]
for vertex, neighbours in self._snapshot_items():
if not list(neighbours):
isolated.append(vertex)
return isolated

def find_path(self, start_vertex: str, end_vertex: str, path=[]) -> list:
Expand Down Expand Up @@ -247,9 +273,10 @@
path_length = sys.maxsize
optimum_path = []
if start_vertex is None:
# We now have to find where to begin.....
for possible_start, vertices in self.__type_dict.items():
if end_vertex in vertices:
# We now have to find where to begin..... Iterate over a snapshot
for possible_start, vertices in self._snapshot_items():
vertices_snapshot = list(vertices)
if end_vertex in vertices_snapshot:
temp_path = self.find_path(possible_start, end_vertex)
if len(temp_path) < path_length:
path_length = len(temp_path)
Expand All @@ -270,7 +297,7 @@
start_vertex = vertices[0]
vertices_encountered.add(start_vertex)
if len(vertices_encountered) != len(vertices):
for vertex in graph[start_vertex]:
for vertex in list(graph[start_vertex]):
if vertex not in vertices_encountered and self.is_connected(vertices_encountered, vertex):
return True
else:
Expand Down
18 changes: 18 additions & 0 deletions tests/unit_tests/global_object/test_map.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,24 @@ def test_find_type_unknown_object(self, clear):
# When/Then
result = global_object.map.find_type(unknown_obj)
assert result is None

def test_returned_objs_access_safe_under_modification(self, clear):
"""Ensure accessing returned_objs doesn't raise when entries change size during iteration."""
objs = [ObjBase(name=f"race_{i}") for i in range(8)]
# Mark all as returned
for o in objs:
global_object.map.change_type(o, 'returned')

# Repeatedly access returned_objs while deleting objects and forcing GC to
# try to trigger concurrent modification. This used to raise RuntimeError.
for _ in range(200):
_ = global_object.map.returned_objs # should not raise
if _ and objs:
# delete one object and collect to trigger finalizer/prune
del objs[0]
gc.collect()
# If we got here without exceptions, consider the access safe
assert True

def test_reset_type(self, clear, base_object):
"""Test resetting object type"""
Expand Down
Loading