fix: исправлена работа FSM-контекстов#93
Conversation
- UserAdded.get_ids() и UserRemoved.get_ids() теперь возвращают self.user.user_id вместо inviter_id/admin_id — ключ FSM-контекста теперь указывает на правильного пользователя - MemoryContext.get_data() возвращает копию dict, устраняя race condition при параллельных хендлерах (use_create_task=True) - Добавлено logger_dp.debug при вытеснении контекста из LRU-кеша
|
@bish-x, пункт 1 - получение ID используется в рамках контекста пользователя, который как раз совершил действие. В случае этого исправления получается так, что приглашенный или удаленный пользователь вступает в контекст, из-за чего получается бессмыслица |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR aims to fix incorrect FSM context keying (so state is bound to the actual affected user), prevent unintended shared mutations of in-memory FSM data across concurrent handlers, and improve observability when FSM contexts are evicted from the dispatcher’s LRU cache.
Changes:
- Added debug logging when an FSM context is evicted from the dispatcher LRU cache due to
CONTEXTS_MAX_SIZE. - Updated
MemoryContext.get_data()to return a copy of the internal dict instead of the internal dict reference.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
maxapi/dispatcher.py |
Adds debug logging for LRU context eviction in __get_context. |
maxapi/context/context.py |
Makes MemoryContext.get_data() return a copy to avoid shared-reference mutations across concurrent handlers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if len(self.contexts) >= CONTEXTS_MAX_SIZE: | ||
| evicted_key = next(iter(self.contexts)) | ||
| logger_dp.debug( | ||
| "Вытеснен контекст %s (лимит %d)", | ||
| evicted_key, | ||
| CONTEXTS_MAX_SIZE, | ||
| ) | ||
| self.contexts.popitem(last=False) |
There was a problem hiding this comment.
PR description states that UserAdded.get_ids() / UserRemoved.get_ids() were fixed to return the affected user id (self.user.user_id), but in the current code they still return inviter_id / admin_id (see maxapi/types/updates/user_added.py and maxapi/types/updates/user_removed.py). As a result, the dispatcher will continue binding FSM contexts to the wrong user via event_object.get_ids(), and the described collision for (chat_id, None) is still possible. Please include the missing fixes (or adjust the PR description if this change is intentionally not part of this PR).
| async with self._lock: | ||
| return self._context | ||
| return self._context.copy() |
There was a problem hiding this comment.
MemoryContext.get_data() now returns a copy; please add a regression test that mutating the dict returned by get_data() does not change the underlying context unless set_data() / update_data() is called. This will lock in the intended concurrency behavior and prevent accidental reintroduction of returning the internal dict reference.
| evicted_key = next(iter(self.contexts)) | ||
| logger_dp.debug( | ||
| "Вытеснен контекст %s (лимит %d)", | ||
| evicted_key, | ||
| CONTEXTS_MAX_SIZE, | ||
| ) | ||
| self.contexts.popitem(last=False) |
There was a problem hiding this comment.
The eviction log currently computes evicted_key = next(iter(self.contexts)) and then calls popitem(last=False). Consider using the return value of popitem(last=False) for the key you log so the log always matches the actual evicted entry and avoids the extra lookup.
| evicted_key = next(iter(self.contexts)) | |
| logger_dp.debug( | |
| "Вытеснен контекст %s (лимит %d)", | |
| evicted_key, | |
| CONTEXTS_MAX_SIZE, | |
| ) | |
| self.contexts.popitem(last=False) | |
| evicted_key, _ = self.contexts.popitem(last=False) | |
| logger_dp.debug( | |
| "Вытеснен контекст %s (лимит %d)", | |
| evicted_key, | |
| CONTEXTS_MAX_SIZE, | |
| ) |
Подтянуты PR из upstream: love-apples#93 (FSM), love-apples#96 (download_file), love-apples#101 (fetch user/chat), love-apples#105 (ClipboardButton), love-apples#109 (share payload), love-apples#110 (webhook secret warning). Конфликт в tests/test_types.py: принят upstream-стиль (явный update_type, разнесённые assert). Сохранены доп. тесты test_get_ids_ignores_inviter_id / test_get_ids_ignores_admin_id — их purpose именно цель PR love-apples#94 (не путать inviter_id/admin_id с user.user_id).
Описание
1.
MemoryContext.get_data()возвращала ссылку на внутренний dictФайл:
context/context.pyget_data()возвращалаself._contextнапрямую. Приuse_create_task=Trueдва параллельных хендлера одного чата получали одну ссылку — мутации из одного хендлера были видны другому без захвата lock (race condition).Исправлено: возвращается
self._context.copy().2. Добавлено логирование при вытеснении контекста из LRU-кеша
Файл:
dispatcher.pyПри достижении лимита
CONTEXTS_MAX_SIZE(10 000) самый старый контекст удалялся без какого-либо уведомления. Пользователь терял FSM-состояние незаметно. Добавленоlogger_dp.debugпри вытеснении.Тестирование