Skip to content

close() / aclose() / __exit__ are no-ops — connections never released #88

Description

@thomasht86

Summary

Client.close(), Client.__exit__, and AsyncClient.aclose() are no-ops — their entire body is del self, which only unbinds a local name. Connections are never released, and a "closed" client keeps working.

Cause

httpr/__init__.py:280-297:

def __exit__(self, *args):
    """Exit context manager and close client."""
    del self

def close(self) -> None:
    ...
    del self

and aclose() at line 698. del self drops one reference held by the method frame; the caller's reference is untouched, so nothing is dropped or shut down.

Reproduction

Against a local keep-alive (HTTP/1.1) server, counting this process's TCP connections to it:

open conns after request:            1
open conns after close():            1     <- not released
client still usable after close():   200   <- request succeeds on a "closed" client
open conns after ctx-manager exit:   2     <- `with httpr.Client() as c:` leaks too

httpx, by contrast, closes the pool and raises RuntimeError: Cannot send a request, as the client has been closed. on use-after-close.

Impact

Any long-running process that creates clients dynamically — per tenant, per credential, per proxy — accumulates idle sockets and file descriptors for the lifetime of the process. The context-manager form is the one the README recommends ("Using context manager (recommended)"), so the leak follows the documented happy path. It also means close() gives a false sense of cleanup in try/finally blocks.

Note this is a leak of idle pooled connections; they are eventually reaped by the OS/peer, so it's a slow bleed rather than an immediate failure. Severity is moderate, but the API is currently lying about what it does.

Expected

close() drops the underlying reqwest client and its connection pool; subsequent use raises rather than silently succeeding.

Proposed fix

Add a close() method on RClient in src/lib.rs that takes the client out of its Arc<Mutex<...>> (e.g. hold Option<reqwest::Client> inside the mutex and set it to None), and have the request path raise a clear error — a new ClientClosed-style exception, or reuse an existing one from src/exceptions.rs — when it finds None.

Then make the Python wrappers call it: Client.close(), Client.__exit__, AsyncClient.aclose() and AsyncClient.__aexit__. AsyncClient should additionally shut down self._executor if it owns one (httpr/__init__.py:684).

Dropping a reqwest::Client closes idle pooled connections; in-flight requests holding their own clone finish normally.

Suggested tests

  • Connection count returns to zero after close() and after context-manager exit.
  • client.get(...) after close() raises.
  • AsyncClient(max_concurrency=4) shuts its executor threads down on aclose().

Size

~1-2 hours.

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions