Skip to content
This repository was archived by the owner on Feb 13, 2026. It is now read-only.
This repository was archived by the owner on Feb 13, 2026. It is now read-only.

Clients do not clean up Requests sessions #64

Closed

Description

@akx

Environment details

  • OS type and version: macOS
  • Python version: 3.9
  • google-cloud-core version: 1.4.3
  • google-cloud-storage version: 1.33.0 (since it's related)

Steps to reproduce

This is a sibling issue to googleapis/google-auth-library-python#658 and as such the below is similar to that issue.

Py.test 6.2 enables hard errors on unhandled thread exceptions and resource errors such as leaking sockets/SSL sockets/FDs/.... I've spent the last couple hours debugging various

ResourceWarning: unclosed <ssl.SSLSocket fd=12, family=AddressFamily.AF_INET, type=SocketKind.SOCK_STREAM, proto=0, laddr=('192.168.1.23', 60522), raddr=('216.58.207.202', 443)>

When initiating (e.g.) a google-cloud-storage storage client with e.g.

        creds = self.get_client_credentials()
        return storage.Client(project=creds.project_id, credentials=creds)

the client object is initialized with no explicit _http object, and the default code path is run, creating a new requests.Session() that will be eventually GC'd but is otherwise not close()d, leading to the above warning (and a dropped connection and socket, which may have implications down the line).

Since google.core.client.Client doesn't implement e.g. the context manager protocol or close(), there's no officially sanctioned way to clean up that session; for a storage Client, I subclassed one as a workaround but it would be nice if Clients had an official close().

        class CloseableStorageClient(storage.Client):
            def __enter__(self):
                return self

            def __exit__(self, *args):
                self._http.close()

Resolution suggestion

At the very least, add close() to Clients; by default it should probably only be something like

def close(self):
    if self._http_internal:
        self._http_internal.close()

, the semantics being a best-effort cleanup of resources allocated by the client, but not irreversible finalization.

With this, Clients could be used with contextlib.closing(). Additionally, one could add a minimal context management protocol implementation á la

            def __enter__(self):
                return self

            def __exit__(self, *args):
                self.close()

If you folks think this is a good way to go, I can certainly write the PR :)

cc @busunkim96 (for having triaged the other issue).

Activity

  1. busunkim96 commented on Dec 22, 2020

    @busunkim96
    Contributor

    Thanks @akx!

    CC @crwilcox as a google-cloud-storage owner.

    We've talked about a close() method for the gapic clients. Ideally the behavior will be similar in both the generated and handwritten clients. googleapis/gapic-generator-python#575

  2. added
    priority: p2Moderately-important priority. Fix may not be included in next release.
    type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.
    on Dec 22, 2020
  3. crwilcox commented on Dec 22, 2020

    @crwilcox
    Contributor
  4. tseaver commented on Jan 13, 2021

    @tseaver
    Contributor

    FWIW, I'd argue that this is a feature request, rather than a bug (not that I'm opposed to adding it).

  5. tseaver commented on Jan 13, 2021

    @tseaver
    Contributor

    I'd also suggest that the existence of contextlib.closing makes it unnecessary to add our own __enter__ / __exit__ methods.

  6. added
    🚨This issue needs some love.
    and removed
    🚨This issue needs some love.
    on Apr 14, 2021
  7. added 3 commits that reference this issue on Jun 11, 2021
    4ce08a5
    6834db7
    44c383a
  8. added a commit that references this issue on Jun 14, 2021
    7387fc8
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    priority: p2Moderately-important priority. Fix may not be included in next release.type: bugError or flaw in code with unintended results or allowing sub-optimal usage patterns.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions