Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[BUG][Python] Asyncio cleanup fails when exceptions are raised #5538

Closed
4 of 6 tasks
TBBle opened this issue Mar 6, 2020 · 3 comments
Closed
4 of 6 tasks

[BUG][Python] Asyncio cleanup fails when exceptions are raised #5538

TBBle opened this issue Mar 6, 2020 · 3 comments

Comments

@TBBle
Copy link

TBBle commented Mar 6, 2020

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • What's the version of OpenAPI Generator used?
  • Have you search for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Bounty to sponsor the fix
Description

When an exception occurs in python --library asyncio generated code, it fails to clean up correctly due to use of the __del__ magic method to clean up the aiohttp ClientSession object.

This is due to using RESTClientObject.__del__ to schedule aiohttp.ClientSession.close with asyncio.ensure_future. __del__ doesn't run until the exception is discarded, and in this repro case, that happens outside the asyncio event loop, and hence raises an exception itself.

openapi-generator version

4.3.0-SNAPSHOT, pulled as follows:

PS C:\Users\paulh> docker pull openapitools/openapi-generator-cli
Using default tag: latest
latest: Pulling from openapitools/openapi-generator-cli
709515475419: Already exists
38a1c0aaa6fd: Already exists
cd134db5e982: Already exists
044751432162: Pull complete
d41fd233bfb5: Pull complete
b9e8842b3734: Pull complete
Digest: sha256:76b6fc8b94789382e1d91319b787545ec5b7f29c3709c2c14bc47eab68d3a871
Status: Downloaded newer image for openapitools/openapi-generator-cli:latest
docker.io/openapitools/openapi-generator-cli:latest
PS C:\Users\paulh> docker image ls openapitools/openapi-generator-cli
REPOSITORY                           TAG                 IMAGE ID            CREATED             SIZE
openapitools/openapi-generator-cli   latest              e74d7eb8269d        29 minutes ago      135MB

I don't believe this is a recent regression, the code hasn't been touched since it was introduced in #107 in 2018.

OpenAPI declaration file content or url

https://raw.githubusercontent.com/openapitools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml

Command line used for generation
docker run --rm -v ${PWD}:/local openapitools/openapi-generator-cli generate -i https://raw.githubusercontent.com/openapitools/openapi-generator/master/modules/openapi-generator/src/test/resources/2_0/petstore.yaml -g python --library asyncio -o /local/out/python
Steps to reproduce

After code-gen, create and run the following Python script:

import asyncio
import sys

sys.path.append("out/python")
import openapi_client
from openapi_client.rest import ApiException

async def main():
    configuration = openapi_client.Configuration()

    with openapi_client.ApiClient(configuration) as api_client:

        api_instance = openapi_client.PetApi(api_client)

        try:
            # Trigger a 404 search result
            result = await api_instance.get_pet_by_id("nosuchpet")
        except ApiException as e:
            print(f"get_pet_by_id failed: {e.reason}")
            assert e.status == 404
            raise e


if __name__ == "__main__":
    try:
        asyncio.run(main())
    except ApiException as e:
        assert e.status == 404
        pass
Actual output
get_pet_by_id failed: Not Found
Exception ignored in: <function RESTClientObject.__del__ at 0x00000260D05EC1E0>
Traceback (most recent call last):
  File "out/python\openapi_client\rest.py", line 89, in __del__
    asyncio.ensure_future(self.pool_manager.close())
  File "C:\Program Files\Python37\lib\asyncio\tasks.py", line 580, in ensure_future
    loop = events.get_event_loop()
  File "C:\Program Files\Python37\lib\asyncio\events.py", line 644, in get_event_loop
    % threading.current_thread().name)
RuntimeError: There is no current event loop in thread 'MainThread'.
sys:1: RuntimeWarning: coroutine 'ClientSession.close' was never awaited
RuntimeWarning: Enable tracemalloc to get the object allocation traceback
Unclosed client session
client_session: <aiohttp.client.ClientSession object at 0x00000260D05F12B0>
Expected output
get_pet_by_id failed: Not Found
Related issues/PRs

None I'm aware of.

Suggest a fix
Workaround

A monkey-patch to remove __del__ and a finally block to directly call aiohttp.ClientSession.close within the asyncio context fixes the issue from the user side:

import asyncio
import sys

sys.path.append("out/python")
import openapi_client
from openapi_client.rest import ApiException

async def main():
    configuration = openapi_client.Configuration()

    with openapi_client.ApiClient(configuration) as api_client:

        api_instance = openapi_client.PetApi(api_client)

        try:
            # Trigger a 404 search result
            result = await api_instance.get_pet_by_id("nosuchpet")
        except ApiException as e:
            print(f"get_pet_by_id failed: {e.reason}")
            assert e.status == 404
            raise e
        finally:
            await api_client.rest_client.pool_manager.close()

def monkey():
    from openapi_client.rest import RESTClientObject

    del RESTClientObject.__del__


monkey()

if __name__ == "__main__":
    try:
        asyncio.run(main())
    except ApiException as e:
        assert e.status == 404
        pass

My ideal solution would be if ApiClient in the asyncio config was an asynchronous context manager, which would mean using it as:

async with openapi_client.ApiClient(configuration) as api_client:

Then its __aexit__ code could call an async def close() on RESTClientObject, which would close its aiohttp.ClientSession.

However, the tornado and urllib implementations of RESTClientObject do not have any particular need to be notified on ApiClient.close, so this would be introducing an asyncio-specific path.

@auto-labeler
Copy link

auto-labeler bot commented Mar 6, 2020

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

@TBBle
Copy link
Author

TBBle commented Apr 29, 2020

I haven't tested it yet, but it looks like #5621 fixed this.

@TBBle
Copy link
Author

TBBle commented Apr 29, 2020

Confirmed fixed with latest OpenAPI-Generator Docker image

PS > docker pull openapitools/openapi-generator-cli
Using default tag: latest
latest: Pulling from openapitools/openapi-generator-cli
709515475419: Already exists
38a1c0aaa6fd: Already exists
cd134db5e982: Already exists
6e1edfcbca40: Pull complete
6fe4a4ff99ff: Pull complete
b0db1e10806b: Pull complete
Digest: sha256:09ba860ec2290e41b9b813a0dae6a71f7c6b5476c8312791ca875f91d6a031d9
Status: Downloaded newer image for openapitools/openapi-generator-cli:latest
docker.io/openapitools/openapi-generator-cli:latest

The same command-line for generation as used above, now reports:

PS > cat .\out\python\.openapi-generator\VERSION
4.3.1-SNAPSHOT

And the repro case passes, with one minor change due to API change from this fix:

    with openapi_client.ApiClient(configuration) as api_client:

is now

    async with openapi_client.ApiClient(configuration) as api_client:

I also confirmed this fix in the 4.3.0 release, repeating the reproduction case using Docker image openapitools/openapi-generator-cli:v4.3.0.

PS> docker pull openapitools/openapi-generator-cli:v4.3.0
v4.3.0: Pulling from openapitools/openapi-generator-cli
709515475419: Already exists
38a1c0aaa6fd: Already exists
cd134db5e982: Already exists
ee81babe4587: Pull complete
c278f8daab97: Pull complete
d11990da2af7: Pull complete
Digest: sha256:5dd4f3ca25bd20d5cd0bae020ded36f003840d00a47f1793f98bf01eb9db0d25
Status: Downloaded newer image for openapitools/openapi-generator-cli:v4.3.0
docker.io/openapitools/openapi-generator-cli:v4.3.0

@TBBle TBBle closed this as completed Apr 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant