From 394e79ce88df7d6550c0c41a1ef2799d5bb5495d Mon Sep 17 00:00:00 2001 From: John McCardle Date: Sat, 7 Mar 2026 23:33:05 -0500 Subject: [PATCH] Fix PythonObjectCache race and document die() iteration (closes #269, closes #273) Add mutex lock to PythonObjectCache::lookup() - cache.find() was unprotected against concurrent modification. Document that entity.die() must not be called during iteration over grid.entities. Co-Authored-By: Claude Opus 4.6 --- src/PythonObjectCache.cpp | 2 +- src/UIEntity.cpp | 10 ++++++++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/PythonObjectCache.cpp b/src/PythonObjectCache.cpp index 4d3e453..6dae7e4 100644 --- a/src/PythonObjectCache.cpp +++ b/src/PythonObjectCache.cpp @@ -37,7 +37,7 @@ void PythonObjectCache::registerObject(uint64_t serial, PyObject* weakref) { PyObject* PythonObjectCache::lookup(uint64_t serial) { if (serial == 0) return nullptr; - // No mutex needed for read - GIL protects PyWeakref_GetRef + std::lock_guard lock(serial_mutex); auto it = cache.find(serial); if (it != cache.end()) { PyObject* obj = nullptr; diff --git a/src/UIEntity.cpp b/src/UIEntity.cpp index 013238a..4a82a37 100644 --- a/src/UIEntity.cpp +++ b/src/UIEntity.cpp @@ -926,7 +926,10 @@ PyMethodDef UIEntity::methods[] = { " state = entity.at((5, 3))\n" " state = entity.at(pos=(5, 3))"}, {"index", (PyCFunction)UIEntity::index, METH_NOARGS, "Return the index of this entity in its grid's entity collection"}, - {"die", (PyCFunction)UIEntity::die, METH_NOARGS, "Remove this entity from its grid"}, + {"die", (PyCFunction)UIEntity::die, METH_NOARGS, + "Remove this entity from its grid.\n\n" + "Warning: Do not call during iteration over grid.entities.\n" + "Modifying the collection during iteration raises RuntimeError."}, {"path_to", (PyCFunction)UIEntity::path_to, METH_VARARGS | METH_KEYWORDS, "path_to(x, y) or path_to(target) -> list\n\n" "Find a path to the target position using Dijkstra pathfinding.\n\n" @@ -997,7 +1000,10 @@ PyMethodDef UIEntity_all_methods[] = { " state = entity.at((5, 3))\n" " state = entity.at(pos=(5, 3))"}, {"index", (PyCFunction)UIEntity::index, METH_NOARGS, "Return the index of this entity in its grid's entity collection"}, - {"die", (PyCFunction)UIEntity::die, METH_NOARGS, "Remove this entity from its grid"}, + {"die", (PyCFunction)UIEntity::die, METH_NOARGS, + "Remove this entity from its grid.\n\n" + "Warning: Do not call during iteration over grid.entities.\n" + "Modifying the collection during iteration raises RuntimeError."}, {"path_to", (PyCFunction)UIEntity::path_to, METH_VARARGS | METH_KEYWORDS, "path_to(x, y) or path_to(target) -> list\n\n" "Find a path to the target position using Dijkstra pathfinding.\n\n"