Skip to content

Commit

Permalink
Remove the __del__ method again from asyincio.Connection
Browse files Browse the repository at this point in the history
  • Loading branch information
kristjanvalur committed Jul 20, 2023
1 parent a80c64c commit 2d8a128
Show file tree
Hide file tree
Showing 2 changed files with 2 additions and 60 deletions.
23 changes: 0 additions & 23 deletions redis/asyncio/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -207,9 +207,6 @@ def __repr__(self):
repr_args = ",".join((f"{k}={v}" for k, v in self.repr_pieces()))
return f"{self.__class__.__name__}<{repr_args}>"

def __del__(self):
self._close_socket()

@abstractmethod
def repr_pieces(self):
pass
Expand Down Expand Up @@ -380,26 +377,6 @@ async def disconnect(self, nowait: bool = False) -> None:
f"Timed out closing connection after {self.socket_connect_timeout}"
) from None

def _close_socket(self):
"""Close the socket directly. Used during garbage collection to
make sure the underlying socket is released. This does not happen
reliably when the stream is garbage collected. This is a safety
precaution, correct use of the library should ensure that
sockets are disconnected properly.
"""
# some test classes don't even have this
writer = getattr(self, "_writer", None)
if writer:
if os.getpid() == self.pid:
try:
writer.close()
except RuntimeError:
# This may fail if the event loop is already closed,
# even though this is not an async call. In this
# case, just ignore the error, since it is during
# exit anyway.
pass

async def _send_ping(self):
"""Send PING, expect PONG in return"""
await self.send_command("PING", check_health=False)
Expand Down
39 changes: 2 additions & 37 deletions tests/test_asyncio/test_connection.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,7 @@
import asyncio
import platform
import socket
import types
from unittest.mock import Mock, patch
from unittest.mock import patch

import pytest
import redis
Expand All @@ -13,12 +12,7 @@
_AsyncRESPBase,
)
from redis.asyncio import Redis
from redis.asyncio.connection import (
AbstractConnection,
Connection,
UnixDomainSocketConnection,
parse_url,
)
from redis.asyncio.connection import Connection, UnixDomainSocketConnection, parse_url
from redis.asyncio.retry import Retry
from redis.backoff import NoBackoff
from redis.exceptions import ConnectionError, InvalidResponse, TimeoutError
Expand Down Expand Up @@ -320,32 +314,3 @@ async def get_redis_connection():
assert r1.auto_close_connection_pool is False
await r1.connection_pool.disconnect()
await r1.close()


@pytest.mark.onlynoncluster
@pytest.mark.parametrize("from_url", (True, False))
async def test_connection_socket_cleanup(request, from_url):
"""Verify that connections are cleaned up when they
are garbage collected
"""
if platform.python_implementation() != "CPython":
pytest.skip("only works on CPython")
url: str = request.config.getoption("--redis-url")
url_args = parse_url(url)

async def get_redis_connection():
if from_url:
return Redis.from_url(url)
return Redis(**url_args)

async def do_something(redis):
await redis.incr("counter")
await redis.close()

mock = Mock()
with patch.object(AbstractConnection, "_close_socket", mock):
r1 = await get_redis_connection()
await do_something(r1)
r1 = None

assert mock.call_count == 1

0 comments on commit 2d8a128

Please sign in to comment.