graphql-python/gql

Hang for 10s on session close when using AIOHTTPTransport with connector_owner=False

shawnlewis opened this issue · 2 comments

Reproduce:

connector = aiohttp.TCPConnector()
transport = AIOHTTPTransport(
    url="...",
    client_session_args={
        "connector": self.connector,
        "connector_owner": False,
    })
async with client = gql.Client(transport=transport, fetch_schema_from_transport=False) as session:
    resp = await session.execute(query, kwargs)

Exiting the client context manager hangs for 10s, the default ssl_close_timeout.

This happens because exiting the context manager waits for a connection closed event that it creates, but it doesn't actually call close on the connection pool since connector_owner is False (see below for details).

aiohttp.py:close

    async def close(self) -> None:
        """Coroutine which will close the aiohttp session.

        Don't call this coroutine directly on the transport, instead use
        :code:`async with` on the client and this coroutine will be executed
        when you exit the async context manager.
        """
        if self.session is not None:

            log.debug("Closing transport")

            closed_event = self.create_aiohttp_closed_event(self.session)
            await self.session.close()
            try:
                await asyncio.wait_for(closed_event.wait(), self.ssl_close_timeout)
            except asyncio.TimeoutError:
                pass
        self.session = None

calls self.session.close()

client.py:close

    async def close(self) -> None:
        """Close underlying connector.

        Release all acquired resources.
        """
        if not self.closed:
            if self._connector is not None and self._connector_owner:
                await self._connector.close()
            self._connector = None

If self._connector_owner is False, we don't call close on the connector.

The closed_event we wait for in aiohttp.py:close is never fired because we don't try to close anything.

Please check the PR #382

Looks good!