Skip to content

Commit

Permalink
Revert "Check socket readability when used from pool"
Browse files Browse the repository at this point in the history
This reverts commit bee3d9a.
  • Loading branch information
MarkusSintonen committed Jun 15, 2024
1 parent bee3d9a commit 4254b13
Show file tree
Hide file tree
Showing 16 changed files with 47 additions and 73 deletions.
5 changes: 0 additions & 5 deletions httpcore/_async/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,6 @@ async def aclose(self) -> None:
async with Trace("close", logger, None, {}):
await self._connection.aclose()

def is_readable(self) -> bool:
if self._connection is None:
return False
return self._connection.is_readable()

def is_available(self) -> bool:
if self._connection is None:
# If HTTP/2 support is enabled, and the resulting connection could
Expand Down
6 changes: 0 additions & 6 deletions httpcore/_async/connection_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,6 @@ async def handle_async_request(self, request: Request) -> Response:
connection = await pool_request.wait_for_connection(timeout=timeout)

try:
if connection.is_readable():
# If the socket is readable, then the only valid state is
# that the socket is about to return b"", indicating
# a server-initiated disconnect.
raise ConnectionNotAvailable

# Send the request on the assigned connection.
response = await connection.handle_async_request(
pool_request.request
Expand Down
22 changes: 18 additions & 4 deletions httpcore/_async/http11.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,6 @@ async def aclose(self) -> None:
def can_handle_request(self, origin: Origin) -> bool:
return origin == self._origin

def is_readable(self) -> bool:
return self._network_stream.get_extra_info("is_readable") # type: ignore[no-any-return]

def is_available(self) -> bool:
# Note that HTTP/1.1 connections in the "NEW" state are not treated as
# being "available". The control flow which created the connection will
Expand All @@ -290,7 +287,24 @@ def is_available(self) -> bool:

def has_expired(self) -> bool:
now = time.monotonic()
return self._expire_at is not None and now > self._expire_at
keepalive_expired = self._expire_at is not None and now > self._expire_at
if keepalive_expired:
return True

# If the HTTP connection is idle but the socket is readable, then the
# only valid state is that the socket is about to return b"", indicating
# a server-initiated disconnect.
# Checking the readable status is relatively expensive so check it at a lower frequency.
if (now - self._network_stream_used_at) > self._socket_poll_interval():
self._network_stream_used_at = now
server_disconnected = (
self._state == HTTPConnectionState.IDLE
and self._network_stream.get_extra_info("is_readable")
)
if server_disconnected:
return True

return False

def _socket_poll_interval(self) -> float:
# Randomize to avoid polling for all the connections at once
Expand Down
3 changes: 0 additions & 3 deletions httpcore/_async/http2.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,9 +505,6 @@ async def _wait_for_outgoing_flow(self, request: Request, stream_id: int) -> int
def can_handle_request(self, origin: Origin) -> bool:
return origin == self._origin

def is_readable(self) -> bool:
return self._network_stream.get_extra_info("is_readable") # type: ignore[no-any-return]

def is_available(self) -> bool:
return (
self._state != HTTPConnectionState.CLOSED
Expand Down
6 changes: 0 additions & 6 deletions httpcore/_async/http_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ async def aclose(self) -> None:
def info(self) -> str:
return self._connection.info()

def is_readable(self) -> bool:
return self._connection.is_readable()

def is_available(self) -> bool:
return self._connection.is_available()

Expand Down Expand Up @@ -355,9 +352,6 @@ async def aclose(self) -> None:
def info(self) -> str:
return self._connection.info()

def is_readable(self) -> bool:
return self._connection.is_readable()

def is_available(self) -> bool:
return self._connection.is_available()

Expand Down
3 changes: 0 additions & 3 deletions httpcore/_async/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ def info(self) -> str:
def can_handle_request(self, origin: Origin) -> bool:
raise NotImplementedError() # pragma: nocover

def is_readable(self) -> bool:
raise NotImplementedError()

def is_available(self) -> bool:
"""
Return `True` if the connection is currently able to accept an
Expand Down
5 changes: 0 additions & 5 deletions httpcore/_async/socks_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,6 @@ async def aclose(self) -> None:
if self._connection is not None:
await self._connection.aclose()

def is_readable(self) -> bool:
if self._connection is None:
return False
return self._connection.is_readable()

def is_available(self) -> bool:
if self._connection is None: # pragma: nocover
# If HTTP/2 support is enabled, and the resulting connection could
Expand Down
6 changes: 3 additions & 3 deletions httpcore/_backends/anyio.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
class AnyIOStream(AsyncNetworkStream):
def __init__(self, stream: anyio.abc.ByteStream) -> None:
self._stream = stream
self._sock = self._stream.extra(anyio.abc.SocketAttribute.raw_socket)

async def read(
self, max_bytes: int, timeout: typing.Optional[float] = None
Expand Down Expand Up @@ -90,9 +89,10 @@ def get_extra_info(self, info: str) -> typing.Any:
if info == "server_addr":
return self._stream.extra(anyio.abc.SocketAttribute.remote_address, None)
if info == "socket":
return self._sock
return self._stream.extra(anyio.abc.SocketAttribute.raw_socket, None)
if info == "is_readable":
return is_socket_readable(self._sock)
sock = self._stream.extra(anyio.abc.SocketAttribute.raw_socket, None)
return is_socket_readable(sock)
return None


Expand Down
14 changes: 8 additions & 6 deletions httpcore/_backends/trio.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import ssl
import typing
from functools import cached_property

import trio

Expand Down Expand Up @@ -87,18 +86,21 @@ def get_extra_info(self, info: str) -> typing.Any:
# Tracked at https://github.com/python-trio/trio/issues/542
return self._stream._ssl_object # type: ignore[attr-defined]
if info == "client_addr":
return self._socket_stream.socket.getsockname()
return self._get_socket_stream().socket.getsockname()
if info == "server_addr":
return self._socket_stream.socket.getpeername()
return self._get_socket_stream().socket.getpeername()
if info == "socket":
return self._socket_stream.socket
stream = self._stream
while isinstance(stream, trio.SSLStream):
stream = stream.transport_stream
assert isinstance(stream, trio.SocketStream)
return stream.socket
if info == "is_readable":
socket = self.get_extra_info("socket")
return socket.is_readable()
return None

@cached_property
def _socket_stream(self) -> trio.SocketStream:
def _get_socket_stream(self) -> trio.SocketStream:
stream = self._stream
while isinstance(stream, trio.SSLStream):
stream = stream.transport_stream
Expand Down
5 changes: 0 additions & 5 deletions httpcore/_sync/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -170,11 +170,6 @@ def close(self) -> None:
with Trace("close", logger, None, {}):
self._connection.close()

def is_readable(self) -> bool:
if self._connection is None:
return False
return self._connection.is_readable()

def is_available(self) -> bool:
if self._connection is None:
# If HTTP/2 support is enabled, and the resulting connection could
Expand Down
6 changes: 0 additions & 6 deletions httpcore/_sync/connection_pool.py
Original file line number Diff line number Diff line change
Expand Up @@ -192,12 +192,6 @@ def handle_request(self, request: Request) -> Response:
connection = pool_request.wait_for_connection(timeout=timeout)

try:
if connection.is_readable():
# If the socket is readable, then the only valid state is
# that the socket is about to return b"", indicating
# a server-initiated disconnect.
raise ConnectionNotAvailable

# Send the request on the assigned connection.
response = connection.handle_request(
pool_request.request
Expand Down
22 changes: 18 additions & 4 deletions httpcore/_sync/http11.py
Original file line number Diff line number Diff line change
Expand Up @@ -278,9 +278,6 @@ def close(self) -> None:
def can_handle_request(self, origin: Origin) -> bool:
return origin == self._origin

def is_readable(self) -> bool:
return self._network_stream.get_extra_info("is_readable") # type: ignore[no-any-return]

def is_available(self) -> bool:
# Note that HTTP/1.1 connections in the "NEW" state are not treated as
# being "available". The control flow which created the connection will
Expand All @@ -290,7 +287,24 @@ def is_available(self) -> bool:

def has_expired(self) -> bool:
now = time.monotonic()
return self._expire_at is not None and now > self._expire_at
keepalive_expired = self._expire_at is not None and now > self._expire_at
if keepalive_expired:
return True

# If the HTTP connection is idle but the socket is readable, then the
# only valid state is that the socket is about to return b"", indicating
# a server-initiated disconnect.
# Checking the readable status is relatively expensive so check it at a lower frequency.
if (now - self._network_stream_used_at) > self._socket_poll_interval():
self._network_stream_used_at = now
server_disconnected = (
self._state == HTTPConnectionState.IDLE
and self._network_stream.get_extra_info("is_readable")
)
if server_disconnected:
return True

return False

def _socket_poll_interval(self) -> float:
# Randomize to avoid polling for all the connections at once
Expand Down
3 changes: 0 additions & 3 deletions httpcore/_sync/http2.py
Original file line number Diff line number Diff line change
Expand Up @@ -505,9 +505,6 @@ def _wait_for_outgoing_flow(self, request: Request, stream_id: int) -> int:
def can_handle_request(self, origin: Origin) -> bool:
return origin == self._origin

def is_readable(self) -> bool:
return self._network_stream.get_extra_info("is_readable") # type: ignore[no-any-return]

def is_available(self) -> bool:
return (
self._state != HTTPConnectionState.CLOSED
Expand Down
6 changes: 0 additions & 6 deletions httpcore/_sync/http_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,6 @@ def close(self) -> None:
def info(self) -> str:
return self._connection.info()

def is_readable(self) -> bool:
return self._connection.is_readable()

def is_available(self) -> bool:
return self._connection.is_available()

Expand Down Expand Up @@ -355,9 +352,6 @@ def close(self) -> None:
def info(self) -> str:
return self._connection.info()

def is_readable(self) -> bool:
return self._connection.is_readable()

def is_available(self) -> bool:
return self._connection.is_available()

Expand Down
3 changes: 0 additions & 3 deletions httpcore/_sync/interfaces.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,6 @@ def info(self) -> str:
def can_handle_request(self, origin: Origin) -> bool:
raise NotImplementedError() # pragma: nocover

def is_readable(self) -> bool:
raise NotImplementedError()

def is_available(self) -> bool:
"""
Return `True` if the connection is currently able to accept an
Expand Down
5 changes: 0 additions & 5 deletions httpcore/_sync/socks_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -306,11 +306,6 @@ def close(self) -> None:
if self._connection is not None:
self._connection.close()

def is_readable(self) -> bool:
if self._connection is None:
return False
return self._connection.is_readable()

def is_available(self) -> bool:
if self._connection is None: # pragma: nocover
# If HTTP/2 support is enabled, and the resulting connection could
Expand Down

0 comments on commit 4254b13

Please sign in to comment.