Repository: XKNX/xknx
Pull request #1192 (2023-02-14 09:23:25)

Don't create unreferenced asyncio Tasks
<!--
  You are awesome! Thanks for contributing to our project!
  Please, DO NOT DELETE ANY TEXT from this template!.
-->
## Description
<!--
Please include a summary of the change and which issue is fixed.
Please also include relevant motivation and context.
List any dependencies that are required for this change.
-->
add `background()` to `TaskRegistry`

see https://textual.textualize.io/blog/2023/02/11/the-heisenbug-lurking-in-your-async-code/

Fixes # (issue)

## Type of change
<!--
Please tick the applicable options.
NOTE: Ticking multiple options most likely indicates
that your change is to big and it is suggested to split it into several smaller PRs.
-->

- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Code quality improvements to existing code or addition of tests

## Checklist

- [ ] The documentation has been adjusted accordingly
- [x] Tests have been added that prove the fix is effective or that the feature works
- [x] The changes are documented in the changelog (docs/changelog.md)

codecov[bot]: # [Codecov](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) Report
> Merging [#1192](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (e8bc74c) into [main](https://codecov.io/gh/XKNX/xknx/commit/88fc1ead4cdb10bdacee6824905540365588bf9d?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) (88fc1ea) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/XKNX/xknx/pull/1192/graphs/tree.svg?width=650&height=150&src=pr&token=irWbIygS84&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX)

```diff
@@           Coverage Diff           @@
##             main    #1192   +/-   ##
=======================================
  Coverage   96.40%   96.40%           
=======================================
  Files         143      143           
  Lines        8949     8954    +5     
=======================================
+ Hits         8627     8632    +5     
  Misses        322      322           
```


| [Impacted Files](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX) | Coverage Δ | |
|---|---|---|
| [xknx/core/task\_registry.py](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9jb3JlL3Rhc2tfcmVnaXN0cnkucHk=) | `100.00% <100.00%> (ø)` | |
| [xknx/devices/cover.py](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9kZXZpY2VzL2NvdmVyLnB5) | `99.46% <100.00%> (ø)` | |
| [xknx/io/tunnel.py](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9pby90dW5uZWwucHk=) | `85.88% <100.00%> (ø)` | |
| [xknx/management/management.py](https://codecov.io/gh/XKNX/xknx/pull/1192?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XKNX#diff-eGtueC9tYW5hZ2VtZW50L21hbmFnZW1lbnQucHk=) | `88.19% <100.00%> (ø)` | |


Patch:
diff --git a/docs/changelog.md b/docs/changelog.md
index 5834d7d3..9beefb37 100644
--- a/docs/changelog.md
+++ b/docs/changelog.md
@@ -19,6 +19,7 @@ nav_order: 2
 - Parse Data Secure credentials from Keyrings exported for specific interfaces.
 - Fix callback for Cover target position when called with same value consecutively.
 - Fix Windows TCP transport bug when using IP Secure Tunnelling.
+- Don't create unreferenced asyncio Tasks. `xknx.task_registry.background()` can now be used to create background tasks.
 
 ### Protocol
 
diff --git a/xknx/core/task_registry.py b/xknx/core/task_registry.py
index 7a753f33..37f34cb9 100644
--- a/xknx/core/task_registry.py
+++ b/xknx/core/task_registry.py
@@ -75,6 +75,8 @@ class TaskRegistry:
         self.xknx = xknx
         self.tasks: list[Task] = []
 
+        self._background_task: set[asyncio.Task[None]] = set()
+
     def register(
         self,
         name: str,
@@ -132,3 +134,13 @@ class TaskRegistry:
                 task.reconnected()
             else:
                 task.connection_lost()
+
+    def background(self, async_func: Coroutine[Any, Any, None]) -> None:
+        """Run a task in the background. This task will not be tracked by the TaskRegistry."""
+        # Add task to the set. This creates a strong reference so it can't be garbage collected.
+        task = asyncio.create_task(async_func)
+        # To prevent keeping references to finished tasks forever,
+        self._background_task.add(task)
+        # make each task remove its own reference from the set after
+        # completion:
+        task.add_done_callback(self._background_task.discard)
diff --git a/xknx/io/tunnel.py b/xknx/io/tunnel.py
index 285867e0..3e4979d0 100644
--- a/xknx/io/tunnel.py
+++ b/xknx/io/tunnel.py
@@ -142,7 +142,7 @@ class _Tunnel(Interface):
     def _tunnel_lost(self) -> None:
         """Prepare for reconnection or shutdown when the connection is lost. Callback."""
         self.stop_heartbeat()
-        asyncio.create_task(
+        self.xknx.task_registry.background(
             self.xknx.connection_manager.connection_state_changed(
                 XknxConnectionState.DISCONNECTED
             )
diff --git a/xknx/management/management.py b/xknx/management/management.py
index 40e31088..dc237762 100644
--- a/xknx/management/management.py
+++ b/xknx/management/management.py
@@ -41,7 +41,9 @@ class Management:
                 destination_address=telegram.source_address,
                 tpci=TAck(sequence_number=telegram.tpci.sequence_number),
             )
-            asyncio.create_task(self.xknx.cemi_handler.send_telegram(ack))
+            self.xknx.task_registry.background(
+                self.xknx.cemi_handler.send_telegram(ack)
+            )
         if conn := self._connections.get(telegram.source_address):
             conn.process(telegram)
             return
@@ -58,7 +60,9 @@ class Management:
             disconnect = Telegram(
                 destination_address=telegram.source_address, tpci=TDisconnect()
             )
-            asyncio.create_task(self.xknx.cemi_handler.send_telegram(disconnect))
+            self.xknx.task_registry.background(
+                self.xknx.cemi_handler.send_telegram(disconnect)
+            )
             return
         logger.warning("Unhandled management telegram: %r", telegram)
         return


Test patch:
diff --git a/test/core_tests/task_registry_test.py b/test/core_tests/task_registry_test.py
index f10e3874..03295144 100644
--- a/test/core_tests/task_registry_test.py
+++ b/test/core_tests/task_registry_test.py
@@ -1,5 +1,6 @@
 """Unit test for task registry."""
 import asyncio
+import sys
 
 from xknx import XKNX
 from xknx.core import XknxConnectionState
@@ -124,3 +125,26 @@ class TestTaskRegistry:
         await asyncio.sleep(0)  # iterate loop to cancel task
         assert self.test == 0
         assert len(xknx.connection_manager._connection_state_changed_cbs) == 0
+
+    async def test_background(self, time_travel):
+        """Test running background task."""
+        TIME = 10
+
+        async def callback() -> None:
+            """Do nothing."""
+            await asyncio.sleep(TIME)
+
+        xknx = XKNX()
+        xknx.task_registry.background(callback())
+        assert len(xknx.task_registry._background_task) == 1
+        task = next(iter(xknx.task_registry._background_task))
+        refs = sys.getrefcount(task)
+        assert refs == 4
+        assert not task.done()
+
+        # after task is finished it should remove itself from the background registry
+        await time_travel(TIME)
+        assert len(xknx.task_registry._background_task) == 0
+        assert task.done()
+        refs = sys.getrefcount(task)
+        assert refs == 2
diff --git a/test/io_tests/tunnel_test.py b/test/io_tests/tunnel_test.py
index 819e4ba4..2c6d1759 100644
--- a/test/io_tests/tunnel_test.py
+++ b/test/io_tests/tunnel_test.py
@@ -323,7 +323,7 @@ class TestUDPTunnel:
                 raw_cemi=test_cemi.to_knx(),
             )
         )
-        asyncio.create_task(self.tunnel.send_cemi(test_cemi))
+        _send_task = asyncio.create_task(self.tunnel.send_cemi(test_cemi))
         await time_travel(0)
         self.tunnel.transport.send.assert_called_once_with(
             test_telegram_frame, addr=data_endpoint_addr