`Unclosed ClusterNode object` error triggered by `valkey.asyncio.cluster.ClusterNode.__del__()`

[jakob-keller]

valkey-py 6.1.0 appears to handle asyncio cluster connections uncleanly. As a result, I am seeing crashes due to Unclosed ClusterNode object errors being raised in the event loop.

I assume this is what triggers it:

1. In valkey.asyncio.cluster.NodesManager.initialize(), .set_nodes() is called with remove_old=True argument, if certain conditions are satisfied:
   

   valkey-py/valkey/asyncio/cluster.py

   Line 1406 in ca5c7c5

   self.set_nodes(self.startup_nodes, self.nodes_cache, remove_old=True)

2. .set_nodes() creates asyncio tasks to disconnect any 'old' cluster nodes, but fails to await those tasks (the noqa pragma might be telling):
   

   valkey-py/valkey/asyncio/cluster.py

   Lines 1199 to 1215 in ca5c7c5

   	def set_nodes(
   	self,
   	old: Dict[str, "ClusterNode"],
   	new: Dict[str, "ClusterNode"],
   	remove_old: bool = False,
   	) -> None:
   	if remove_old:
   	for name in list(old.keys()):
   	if name not in new:
   	task = asyncio.create_task(old.pop(name).disconnect()) # noqa
   	for name, node in new.items():
   	if name in old:
   	if old[name] is node:
   	continue
   	task = asyncio.create_task(old[name].disconnect()) # noqa
   	old[name] = node

3. The finalizer of valkey.asyncio.cluster.ClusterNode checks for any connections that have not been disconnected yet. Since the disconnection tasks from 2. are not awaited, the failure is triggered:
   

   valkey-py/valkey/asyncio/cluster.py

   Lines 1022 to 1036 in ca5c7c5

   	def __del__(
   	self,
   	_warn: Any = warnings.warn,
   	_grl: Any = asyncio.get_running_loop,
   	) -> None:
   	for connection in self._connections:
   	if connection.is_connected:
   	_warn(f"{self._DEL_MESSAGE} {self!r}", ResourceWarning, source=self)
   	try:
   	context = {"client": self, "message": self._DEL_MESSAGE}
   	_grl().call_exception_handler(context)
   	except RuntimeError:
   	pass
   	break

As a potential solution, set_nodes() could be made into a coroutine function that awaits the disconnect tasks before returning.

Client: valkey-py 6.1.0, CPython 3.13.2, containerized Linux on arm64
Database: Amazon ElastiCache for Valkey 8.0.1, t4g based instances, cluster mode enabled

[mkmkme]

Hey!

Thanks for the issue. Do you happen to have a minimal reproducer for this error you're facing?

[jakob-keller]

Unfortunately, not at this point. The issue is triggered as part of a complex, proprietary service and could be conditional on system load and/or a high number of concurrent requests being processed.

However, I believe the way asyncio.create_task() is used here (as well as in other parts of the codebase!) is highly problematic. I am pretty sure this anti-pattern causes the premature finalization to occur.

https://docs.python.org/3/library/asyncio-task.html#asyncio.create_task:

Important: Save a reference to the result of this function, to avoid a task disappearing mid-execution. The event loop only keeps weak references to tasks. A task that isn’t referenced elsewhere may get garbage collected at any time, even before it’s done. For reliable “fire-and-forget” background tasks, gather them in a collection:

background_tasks = set()

for i in range(10):
    task = asyncio.create_task(some_coro(param=i))

    # Add task to the set. This creates a strong reference.
    background_tasks.add(task)

    # To prevent keeping references to finished tasks forever,
    # make each task remove its own reference from the set after
    # completion:
    task.add_done_callback(background_tasks.discard)

[jakob-keller]

After some more digging, I believe this particular issue might have been introduced recently in 4be949a. That would mean, valkey-py < 6.1.0 is not affected.

Turns out that change did not alter behavior in any way.

[jakob-keller]

It looks like the underlying issue is a long-standing one. 56b254e tried to address it, but is inadequate IMO. I stand by my comment from 2023: redis/redis-py#2816 (comment)

[ronenlinx]

Im encountering this issue aswell in our production environment without really understanding why. Any suggestions ?
Using v6.0.2