Bug Report Checklist
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
passActual 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.
Bug Report Checklist
Have you validated the input using an OpenAPI validator (example)?[Optional] Bounty to sponsor the fixDescription
When an exception occurs in python
--library asynciogenerated code, it fails to clean up correctly due to use of the__del__magic method to clean up the aiohttpClientSessionobject.This is due to using
RESTClientObject.__del__to scheduleaiohttp.ClientSession.closewithasyncio.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:
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
Steps to reproduce
After code-gen, create and run the following Python script:
Actual output
Expected output
Related issues/PRs
None I'm aware of.
Suggest a fix
Workaround
A monkey-patch to remove
__del__and afinallyblock to directly callaiohttp.ClientSession.closewithin the asyncio context fixes the issue from the user side:My ideal solution would be if
ApiClientin the asyncio config was an asynchronous context manager, which would mean using it as:Then its
__aexit__code could call anasync def close()onRESTClientObject, which would close itsaiohttp.ClientSession.However, the tornado and urllib implementations of
RESTClientObjectdo not have any particular need to be notified onApiClient.close, so this would be introducing an asyncio-specific path.