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

Cleanup context is not always run #547

Closed
haukex opened this issue Apr 17, 2023 · 7 comments
Closed

Cleanup context is not always run #547

haukex opened this issue Apr 17, 2023 · 7 comments

Comments

@haukex
Copy link
Contributor

haukex commented Apr 17, 2023

  • aiohttp-devtools version: 1.0.post0
  • aiohttp version: 3.8.4
  • python version: 3.11.3
  • Platform: Windows 10 and Linux

I have discovered two cases in which the second half of a Cleanup Context after the yield is not run. The first is in this issue, and the second is in #548.

The documentation says:

aiohttp guarantees that cleanup code is called if and only if startup code was successfully finished.

I have this simple code, which just has one on_startup, one on_shutdown, and one cleanup_ctx handler.

from aiohttp import web

routes = web.RouteTableDef()

@routes.get('/')
async def get_events(_request):
    return web.Response(text="Hello, world")

app = web.Application()
async def startup(_app):
    print("====> STARTUP", flush=True)
app.on_startup.append(startup)
async def context(_app):
    print("====> BEFORE", flush=True)
    yield
    print("=====> AFTER", flush=True)
    with open('test.log','a') as fh: fh.write("Hello\n")
app.cleanup_ctx.append(context)
async def shutdown(_app):
    print("=====> SHUTDOWN", flush=True)
app.on_shutdown.append(shutdown)
app.add_routes(routes)

if __name__ == '__main__':
    web.run_app(app)

The following output is the same on both Windows 10 and Linux - I run the server using adev runserver, wait a little bit, and then end the server with Ctrl-C.

$ adev runserver --no-livereload testserv.py
[18:45:56] Starting aux server at http://localhost:8001 ◆
[18:45:56] Starting dev server at http://localhost:8000 ●
======== Running on http://0.0.0.0:8001 ========
(Press CTRL+C to quit)
====> BEFORE
====> STARTUP
^C=====> SHUTDOWN
$ cat test.log
cat: test.log: No such file or directory

This shows that the second half of the cleanup context is not run, as I expected it should be.

@Dreamsorcerer
Copy link
Member

Dreamsorcerer commented Apr 17, 2023

With your example, I get:

======== Running on http://0.0.0.0:8001 ========
(Press CTRL+C to quit)
====> BEFORE
====> STARTUP
^C=====> SHUTDOWN
=====> AFTER

If you want to look at the code and try debugging, cleanup should be run here:
https://github.com/aio-libs/aiohttp-devtools/blob/master/aiohttp_devtools/runserver/serve.py#L135
Though both shutdown and cleanup_ctx are handled in the same call, so you'll likely need to dig into aiohttp to see what's happening. Cleanup_ctx is run at: https://github.com/aio-libs/aiohttp/blob/master/aiohttp/web_runner.py#L325
The previous line (site.stop()) will run on_shutdown.

@haukex
Copy link
Contributor Author

haukex commented Apr 18, 2023

Thanks for the quick reply! I will try debugging a bit further and get back to you.

@haukex
Copy link
Contributor Author

haukex commented Apr 18, 2023

Just some quick initial results:

  • On Linux, I am not able to reproduce the issue with the master branch, but I am able to reproduce it with v1.0.post0, so one of the commits in the meantime seems to have fixed the issue (at the moment I don't have enough time to bisect which one it was).

  • On Windows, I am still able to reproduce the issue even with the latest master branch. The "AFTER" is sometimes printed, and sometimes not, but the file is never written - it seems to me like the process is terminating before everything has finished.

Perhaps you could publish a new release, so that the issue is fixed at least on Linux?

@Dreamsorcerer
Copy link
Member

* On Linux, I am not able to reproduce the issue with the `master` branch, but I am able to reproduce it with `v1.0.post0`, so one of the commits in the meantime seems to have fixed the issue (at the moment I don't have enough time to bisect which one it was).

Did you checkout the release tag, just to verify it is not some difference between a pip-installed version and a local one?

The only change that I can even remotely imagine affecting this is the switch to asyncio.Runner() in Python 3.11:
22929a4

@haukex
Copy link
Contributor Author

haukex commented Apr 18, 2023

Did you checkout the release tag, just to verify it is not some difference between a pip-installed version and a local one?

Yes, it was a checkout of the tag v1.0.post0, run in a Python 3.11.3 venv, run via python -c "from aiohttp_devtools.cli import runserver; runserver()" testserv.py.

Unfortunately I don't know enough about asyncio yet, I've only been working with it for a month or so, so I can't say much about the runner, other than that I might have the time to run a bisect in the next few days.

@haukex
Copy link
Contributor Author

haukex commented Apr 18, 2023

Actually, running the bisect wasn't too difficult, and I can confirm that 22929a4 seems to have fixed the issue on Linux.

Is there any chance of a new release?

@Dreamsorcerer
Copy link
Member

I'll look at it next week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants