python-telegram-bot · Bibo-Joshi · Jun 4, 2025 · May 25, 2025 · May 25, 2025 · May 25, 2025
diff --git a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
@@ -0,0 +1,14 @@
+bugfixes = """
+Fixed a bug where calling ``Application.remove/add_handler`` during update handling can cause a ``RuntimeError`` in ``Application.process_update``.
+
+.. hint::
+    Calling ``Application.add/remove_handler`` now has no influence on calls to :meth:`process_update` that are
+    already in progress. The same holds for ``Application.add/remove_error_handler`` and ``Application.process_error``, respectively.
+
+    .. warning::
+        This behavior should currently be considered an implementation detail and not as guaranteed behavior.
+"""
+[[pull_requests]]
+uid = "4802"
+author_uid = "Bibo-Joshi"
+closes_threads = ["4803"]
diff --git a/src/telegram/ext/_application.py b/src/telegram/ext/_application.py
@@ -1256,8 +1256,14 @@ async def process_update(self, update: object) -> None:
         context = None
         any_blocking = False  # Flag which is set to True if any handler specifies block=True
 
-        for handlers in self.handlers.values():
+        # We copy the lists to avoid issues with concurrent modification of the
+        # handlers (groups or handlers in groups) while iterating over it via add/remove_handler.
+        # Currently considered implementation detail as described in docstrings of
+        # add/remove_handler
+        # do *not* use `copy.deepcopy` here, as we don't want to deepcopy the handlers themselves
+        for handlers in [v.copy() for v in self.handlers.values()]:
             try:
+                # no copy needed b/c we copy above
                 for handler in handlers:
                     check = handler.check_update(update)  # Should the handler handle this update?
                     if check is None or check is False:
@@ -1343,6 +1349,14 @@ def add_handler(self, handler: BaseHandler[Any, CCT, Any], group: int = DEFAULT_
             might lead to race conditions and undesired behavior. In particular, current
             conversation states may be overridden by the loaded data.
 
+        Hint:
+            This method currently has no influence on calls to :meth:`process_update` that are
+            already in progress.
+
+            .. warning::
+                This behavior should currently be considered an implementation detail and not as
+                guaranteed behavior.
+
         Args:
             handler (:class:`telegram.ext.BaseHandler`): A BaseHandler instance.
             group (:obj:`int`, optional): The group identifier. Default is ``0``.
@@ -1444,6 +1458,14 @@ def remove_handler(
     ) -> None:
         """Remove a handler from the specified group.
 
+        Hint:
+            This method currently has no influence on calls to :meth:`process_update` that are
+            already in progress.
+
+            .. warning::
+                This behavior should currently be considered an implementation detail and not as
+                guaranteed behavior.
+
         Args:
             handler (:class:`telegram.ext.BaseHandler`): A :class:`telegram.ext.BaseHandler`
                 instance.
@@ -1774,6 +1796,14 @@ def add_error_handler(
         Examples:
             :any:`Errorhandler Bot <examples.errorhandlerbot>`
 
+        Hint:
+            This method currently has no influence on calls to :meth:`process_error` that are
+            already in progress.
+
+            .. warning::
+                This behavior should currently be considered an implementation detail and not as
+                guaranteed behavior.
+
         .. seealso:: :wiki:`Exceptions, Warnings and Logging <Exceptions%2C-Warnings-and-Logging>`
 
         Args:
@@ -1797,6 +1827,14 @@ async def callback(update: Optional[object], context: CallbackContext)
     def remove_error_handler(self, callback: HandlerCallback[object, CCT, None]) -> None:
         """Removes an error handler.
 
+        Hint:
+            This method currently has no influence on calls to :meth:`process_error` that are
+            already in progress.
+
+            .. warning::
+                This behavior should currently be considered an implementation detail and not as
+                guaranteed behavior.
+
         Args:
             callback (:term:`coroutine function`): The error handler to remove.
 
@@ -1838,10 +1876,12 @@ async def process_error(
             :class:`telegram.ext.ApplicationHandlerStop`. :obj:`False`, otherwise.
         """
         if self.error_handlers:
-            for (
-                callback,
-                block,
-            ) in self.error_handlers.items():
+            # We copy the list to avoid issues with concurrent modification of the
+            # error handlers while iterating over it via add/remove_error_handler.
+            # Currently considered implementation detail as described in docstrings of
+            # add/remove_error_handler
+            error_handler_items = list(self.error_handlers.items())
+            for callback, block in error_handler_items:
                 try:
                     context = self.context_types.context.from_error(
                         update=update,

diff --git a/tests/ext/test_application.py b/tests/ext/test_application.py
@@ -2583,6 +2583,70 @@ async def callback(update, context):
             assert received_updates == {2}
             assert len(caplog.records) == 0
 
+    @pytest.mark.parametrize("change_type", ["remove", "add"])
+    async def test_process_update_handler_change_groups_during_iteration(self, app, change_type):
+        run_groups = set()
+
+        async def dummy_callback(_, __, g: int):
+            run_groups.add(g)
+
+        for group in range(10, 20):
+            handler = TypeHandler(int, functools.partial(dummy_callback, g=group))
+            app.add_handler(handler, group=group)
+
+        async def wait_callback(_, context):
+            # Trigger a change of the app.handlers dict during the iteration
+            if change_type == "remove":
+                context.application.remove_handler(handler, group)
+            else:
+                context.application.add_handler(
+                    TypeHandler(int, functools.partial(dummy_callback, g=42)), group=42
+                )
+
+        app.add_handler(TypeHandler(int, wait_callback))
+
+        async with app:
+            await app.process_update(1)
+
+        # check that exactly those handlers were called that were configured when
+        # process_update was called
+        assert run_groups == set(range(10, 20))
+
+    async def test_process_update_handler_change_group_during_iteration(self, app):
+        async def dummy_callback(_, __):
+            pass
+
+        checked_handlers = set()
+
+        class TrackHandler(TypeHandler):
+            def __init__(self, name: str, *args, **kwargs):
+                self.name = name
+                super().__init__(*args, **kwargs)
+
+            def check_update(self, update: object) -> bool:
+                checked_handlers.add(self.name)
+                return super().check_update(update)
+
+        remove_handler = TrackHandler("remove", int, dummy_callback)
+        add_handler = TrackHandler("add", int, dummy_callback)
+
+        class TriggerHandler(TypeHandler):
+            def check_update(self, update: object) -> bool:
+                # Trigger a change of the app.handlers *in the same group* during the iteration
+                app.remove_handler(remove_handler)
+                app.add_handler(add_handler)
+                # return False to ensure that additional handlers in the same group are checked
+                return False
+
+        app.add_handler(TriggerHandler(str, dummy_callback))
+        app.add_handler(remove_handler)
+        async with app:
+            await app.process_update("string update")
+
+        # check that exactly those handlers were checked that were configured when
+        # process_update was called
+        assert checked_handlers == {"remove"}
+
     async def test_process_error_exception_in_building_context(self, monkeypatch, caplog, app):
         # Makes sure that exceptions in building the context don't stop the application
         exception = ValueError("TestException")
@@ -2622,3 +2686,28 @@ async def callback(update, context):
 
             assert received_errors == {2}
             assert len(caplog.records) == 0
+
+    @pytest.mark.parametrize("change_type", ["remove", "add"])
+    async def test_process_error_change_during_iteration(self, app, change_type):
+        called_handlers = set()
+
+        async def dummy_process_error(name: str, *_, **__):
+            called_handlers.add(name)
+
+        add_error_handler = functools.partial(dummy_process_error, "add_handler")
+        remove_error_handler = functools.partial(dummy_process_error, "remove_handler")
+
+        async def trigger_change(*_, **__):
+            if change_type == "remove":
+                app.remove_error_handler(remove_error_handler)
+            else:
+                app.add_error_handler(add_error_handler)
+
+        app.add_error_handler(trigger_change)
+        app.add_error_handler(remove_error_handler)
+        async with app:
+            await app.process_error(update=None, error=None)
+
+        # check that exactly those handlers were checked that were configured when
+        # add_error_handler was called
+        assert called_handlers == {"remove_handler"}