From 4a3d51a5d84ac4d73330512533ee66fb26535823 Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Sun, 25 May 2025 20:11:32 +0200
Subject: [PATCH 1/9] Ensure Safe Handler Looping in
 `Application.process_update`

---
 .../4801.RMLufX4UazobYg5aZojyoD.toml          |  5 ++++
 src/telegram/ext/_application.py              |  4 +++-
 tests/ext/test_application.py                 | 23 +++++++++++++++++++
 3 files changed, 31 insertions(+), 1 deletion(-)
 create mode 100644 changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml

diff --git a/changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml b/changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml
new file mode 100644
index 00000000000..22248f29686
--- /dev/null
+++ b/changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml
@@ -0,0 +1,5 @@
+bugfixes = "Fixed a bug where calling ``Application.remove/add_handler`` during update handling can cause a ``RuntimeError`` in ``Application.process_update``. This is now prevented by raising a ``RuntimeError`` in ``Application.add_handler`` and ``Application.remove_handler`` if the application is already processing an update."
+
+[[pull_requests]]
+uid = "4802"
+author_uid = "Bibo-Joshi"
diff --git a/src/telegram/ext/_application.py b/src/telegram/ext/_application.py
index e856fa85321..a759c58d7cb 100644
--- a/src/telegram/ext/_application.py
+++ b/src/telegram/ext/_application.py
@@ -1256,7 +1256,9 @@ 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 values to a list to avoid issues with concurrent modification of the
+        # handlers dict while iterating over it via add/remove_handler.
+        for handlers in list(self.handlers.values()):
             try:
                 for handler in handlers:
                     check = handler.check_update(update)  # Should the handler handle this update?
diff --git a/tests/ext/test_application.py b/tests/ext/test_application.py
index f375559eb3a..545968d6c9d 100644
--- a/tests/ext/test_application.py
+++ b/tests/ext/test_application.py
@@ -2583,6 +2583,29 @@ 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_during_iteration(self, app, change_type):
+        async def dummy_callback(_, __):
+            pass
+
+        for group in range(10, 20):
+            handler = TypeHandler(int, dummy_callback)
+            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(str, dummy_callback), group=42)
+
+        app.add_handler(TypeHandler(str, wait_callback))
+
+        async with app:
+            # No assertion here. We simply ensure that changing the handler dict size during
+            # the loop inside process_update doesn't raise an exception
+            await app.process_update("string update")
+
     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")

From bbeb0770cfe04e5f3382e61584ffbd2ff63bc62a Mon Sep 17 00:00:00 2001
From: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Sun, 25 May 2025 18:13:42 +0000
Subject: [PATCH 2/9] Add chango fragment for PR #4802

---
 changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml | 5 +++++
 1 file changed, 5 insertions(+)
 create mode 100644 changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml

diff --git a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
new file mode 100644
index 00000000000..2dd1800d8b3
--- /dev/null
+++ b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
@@ -0,0 +1,5 @@
+other = "Ensure Safe Handler Looping in `Application.process_update`"
+[[pull_requests]]
+uid = "4802"
+author_uid = "Bibo-Joshi"
+closes_threads = []

From 4b577d256907edfb5025d9f6443ee994bb583a13 Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Sun, 25 May 2025 20:14:53 +0200
Subject: [PATCH 3/9] chango fix

---
 changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml | 5 -----
 changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml | 4 ++--
 2 files changed, 2 insertions(+), 7 deletions(-)
 delete mode 100644 changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml

diff --git a/changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml b/changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml
deleted file mode 100644
index 22248f29686..00000000000
--- a/changes/unreleased/4801.RMLufX4UazobYg5aZojyoD.toml
+++ /dev/null
@@ -1,5 +0,0 @@
-bugfixes = "Fixed a bug where calling ``Application.remove/add_handler`` during update handling can cause a ``RuntimeError`` in ``Application.process_update``. This is now prevented by raising a ``RuntimeError`` in ``Application.add_handler`` and ``Application.remove_handler`` if the application is already processing an update."
-
-[[pull_requests]]
-uid = "4802"
-author_uid = "Bibo-Joshi"
diff --git a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
index 2dd1800d8b3..22248f29686 100644
--- a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
+++ b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
@@ -1,5 +1,5 @@
-other = "Ensure Safe Handler Looping in `Application.process_update`"
+bugfixes = "Fixed a bug where calling ``Application.remove/add_handler`` during update handling can cause a ``RuntimeError`` in ``Application.process_update``. This is now prevented by raising a ``RuntimeError`` in ``Application.add_handler`` and ``Application.remove_handler`` if the application is already processing an update."
+
 [[pull_requests]]
 uid = "4802"
 author_uid = "Bibo-Joshi"
-closes_threads = []

From 8f349df2ade8e840b66e1ba53b068de83131dab7 Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Mon, 26 May 2025 18:31:59 +0200
Subject: [PATCH 4/9] Link closed issue in chango fragment

---
 changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml | 1 +
 1 file changed, 1 insertion(+)

diff --git a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
index 22248f29686..10ca747f050 100644
--- a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
+++ b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
@@ -3,3 +3,4 @@ bugfixes = "Fixed a bug where calling ``Application.remove/add_handler`` during
 [[pull_requests]]
 uid = "4802"
 author_uid = "Bibo-Joshi"
+closes_threads = ["4803"]

From 9161e9aad3fb0d25d2c56070b97514328d41b2eb Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Mon, 26 May 2025 22:06:12 +0200
Subject: [PATCH 5/9] Extend scope of PR

* cover also process-error
* cover non-critical list-loops
* document behavior as implementation detail
---
 src/telegram/ext/_application.py | 51 +++++++++++++++++++++++----
 tests/ext/test_application.py    | 59 +++++++++++++++++++++++++++++++-
 2 files changed, 102 insertions(+), 8 deletions(-)

diff --git a/src/telegram/ext/_application.py b/src/telegram/ext/_application.py
index a759c58d7cb..a527c23725e 100644
--- a/src/telegram/ext/_application.py
+++ b/src/telegram/ext/_application.py
@@ -1256,10 +1256,13 @@ 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
 
-        # We copy the values to a list to avoid issues with concurrent modification of the
-        # handlers dict while iterating over it via add/remove_handler.
-        for handlers in list(self.handlers.values()):
+        # We deepcopy the dict 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
+        for handlers in deepcopy(self.handlers).values():
             try:
+                # no copy needed b/c we deepcopy above
                 for handler in handlers:
                     check = handler.check_update(update)  # Should the handler handle this update?
                     if check is None or check is False:
@@ -1345,6 +1348,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``.
@@ -1446,6 +1457,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.
@@ -1776,6 +1795,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:
@@ -1799,6 +1826,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.
 
@@ -1840,10 +1875,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
index 545968d6c9d..141c17b9102 100644
--- a/tests/ext/test_application.py
+++ b/tests/ext/test_application.py
@@ -2584,7 +2584,7 @@ async def callback(update, context):
             assert len(caplog.records) == 0
 
     @pytest.mark.parametrize("change_type", ["remove", "add"])
-    async def test_process_update_handler_change_during_iteration(self, app, change_type):
+    async def test_process_update_handler_change_groups_during_iteration(self, app, change_type):
         async def dummy_callback(_, __):
             pass
 
@@ -2606,6 +2606,41 @@ async def wait_callback(_, context):
             # the loop inside process_update doesn't raise an exception
             await app.process_update("string update")
 
+    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")
@@ -2645,3 +2680,25 @@ async def callback(update, context):
 
             assert received_errors == {2}
             assert len(caplog.records) == 0
+
+    async def test_process_error_change_during_iteration(self, app):
+        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(*_, **__):
+            app.add_error_handler(add_error_handler)
+            app.remove_error_handler(remove_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"}

From 5b819ac333b0e72ec9a7c9bad0df5cf9c4085e6d Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Mon, 26 May 2025 22:20:58 +0200
Subject: [PATCH 6/9] extend a test

---
 tests/ext/test_application.py | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/tests/ext/test_application.py b/tests/ext/test_application.py
index 141c17b9102..c99a1311d27 100644
--- a/tests/ext/test_application.py
+++ b/tests/ext/test_application.py
@@ -2585,11 +2585,13 @@ async def callback(update, context):
 
     @pytest.mark.parametrize("change_type", ["remove", "add"])
     async def test_process_update_handler_change_groups_during_iteration(self, app, change_type):
-        async def dummy_callback(_, __):
-            pass
+        run_groups = set()
+
+        async def dummy_callback(_, __, g: int):
+            run_groups.add(g)
 
         for group in range(10, 20):
-            handler = TypeHandler(int, dummy_callback)
+            handler = TypeHandler(int, functools.partial(dummy_callback, g=group))
             app.add_handler(handler, group=group)
 
         async def wait_callback(_, context):
@@ -2597,14 +2599,18 @@ async def wait_callback(_, context):
             if change_type == "remove":
                 context.application.remove_handler(handler, group)
             else:
-                context.application.add_handler(TypeHandler(str, dummy_callback), group=42)
+                context.application.add_handler(
+                    TypeHandler(int, functools.partial(dummy_callback, g=42)), group=42
+                )
 
-        app.add_handler(TypeHandler(str, wait_callback))
+        app.add_handler(TypeHandler(int, wait_callback))
 
         async with app:
-            # No assertion here. We simply ensure that changing the handler dict size during
-            # the loop inside process_update doesn't raise an exception
-            await app.process_update("string update")
+            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(_, __):
@@ -2681,7 +2687,8 @@ async def callback(update, context):
             assert received_errors == {2}
             assert len(caplog.records) == 0
 
-    async def test_process_error_change_during_iteration(self, app):
+    @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, *_, **__):
@@ -2691,8 +2698,10 @@ async def dummy_process_error(name: str, *_, **__):
         remove_error_handler = functools.partial(dummy_process_error, "remove_handler")
 
         async def trigger_change(*_, **__):
-            app.add_error_handler(add_error_handler)
-            app.remove_error_handler(remove_error_handler)
+            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)

From 2373596bc3f973bab8a90de00541ab5bd98d8267 Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Mon, 26 May 2025 22:45:28 +0200
Subject: [PATCH 7/9] fix deepcopy issue

---
 src/telegram/ext/_application.py | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/src/telegram/ext/_application.py b/src/telegram/ext/_application.py
index a527c23725e..58eea6870c4 100644
--- a/src/telegram/ext/_application.py
+++ b/src/telegram/ext/_application.py
@@ -1256,13 +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
 
-        # We deepcopy the dict to avoid issues with concurrent modification of the
+        # We "deepcopy" the dict 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
-        for handlers in deepcopy(self.handlers).values():
+        # 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 deepcopy above
+                # 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:

From 19903e0092295f55ef989ed2a288aefbef783f2f Mon Sep 17 00:00:00 2001
From: Bibo-Joshi <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Thu, 29 May 2025 00:13:43 +0200
Subject: [PATCH 8/9] update comment

Co-authored-by: Abdelrahman Elkheir <90580077+aelkheir@users.noreply.github.com>
---
 src/telegram/ext/_application.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/telegram/ext/_application.py b/src/telegram/ext/_application.py
index 58eea6870c4..336bbb2de65 100644
--- a/src/telegram/ext/_application.py
+++ b/src/telegram/ext/_application.py
@@ -1256,7 +1256,7 @@ 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
 
-        # We "deepcopy" the dict to avoid issues with concurrent modification of the
+        # 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

From 1dffd82996090ec52bad36e24082f18eee70048c Mon Sep 17 00:00:00 2001
From: Hinrich Mahler <22366557+Bibo-Joshi@users.noreply.github.com>
Date: Thu, 29 May 2025 12:37:19 +0200
Subject: [PATCH 9/9] Elaborate chango fragment and fix docstring rendering

---
 changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml | 10 +++++++++-
 src/telegram/ext/_application.py                    |  8 ++++----
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
index 10ca747f050..28745b0d9a8 100644
--- a/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
+++ b/changes/unreleased/4802.RMLufX4UazobYg5aZojyoD.toml
@@ -1,5 +1,13 @@
-bugfixes = "Fixed a bug where calling ``Application.remove/add_handler`` during update handling can cause a ``RuntimeError`` in ``Application.process_update``. This is now prevented by raising a ``RuntimeError`` in ``Application.add_handler`` and ``Application.remove_handler`` if the application is already processing an update."
+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"
diff --git a/src/telegram/ext/_application.py b/src/telegram/ext/_application.py
index 336bbb2de65..c48cd769918 100644
--- a/src/telegram/ext/_application.py
+++ b/src/telegram/ext/_application.py
@@ -1353,7 +1353,7 @@ def add_handler(self, handler: BaseHandler[Any, CCT, Any], group: int = DEFAULT_
             This method currently has no influence on calls to :meth:`process_update` that are
             already in progress.
 
-            Warning:
+            .. warning::
                 This behavior should currently be considered an implementation detail and not as
                 guaranteed behavior.
 
@@ -1462,7 +1462,7 @@ def remove_handler(
             This method currently has no influence on calls to :meth:`process_update` that are
             already in progress.
 
-            Warning:
+            .. warning::
                 This behavior should currently be considered an implementation detail and not as
                 guaranteed behavior.
 
@@ -1800,7 +1800,7 @@ def add_error_handler(
             This method currently has no influence on calls to :meth:`process_error` that are
             already in progress.
 
-            Warning:
+            .. warning::
                 This behavior should currently be considered an implementation detail and not as
                 guaranteed behavior.
 
@@ -1831,7 +1831,7 @@ def remove_error_handler(self, callback: HandlerCallback[object, CCT, None]) ->
             This method currently has no influence on calls to :meth:`process_error` that are
             already in progress.
 
-            Warning:
+            .. warning::
                 This behavior should currently be considered an implementation detail and not as
                 guaranteed behavior.