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

Race Condition Prevents HTTP Server from Exiting #347

Closed
TheTechromancer opened this issue Jul 21, 2024 · 3 comments
Closed

Race Condition Prevents HTTP Server from Exiting #347

TheTechromancer opened this issue Jul 21, 2024 · 3 comments

Comments

@TheTechromancer
Copy link

TheTechromancer commented Jul 21, 2024

Hi, first off thanks for making this library; it's very useful :)

For some time now, I've been running into an elusive race condition that prevents the pytest process from exiting. It only happens in rare cases after cancellation of async tasks, and it's been a real pain to track down.

At first I thought it was a race condition in httpx, since I've discovered similar bugs in the past. But in this case, the issue seems to be specific to pytest-httpserver.

Here is the code to reproduce:

import asyncio
import httpx
import traceback
from pytest_httpserver import HTTPServer


async def make_request(client, url):
    try:
        await client.get(url)
    except Exception:
        print(f"Request failed: {traceback.format_exc()}")


async def main():
    with HTTPServer() as httpserver:
        httpserver.expect_request("/").respond_with_data("")

        async with httpx.AsyncClient() as client:
            print('Creating tasks')
            tasks = []
            for i in range(100):
                tasks.append(asyncio.create_task(make_request(client, httpserver.url_for("/"))))

            print('Cancelling tasks')
            for task in tasks:
                await asyncio.sleep(0)
                task.cancel()

            print('Awaiting tasks')
            for i, task in enumerate(tasks):
                try:
                    await task
                except asyncio.CancelledError:
                    pass
            print('Finished awaiting tasks')
        print('Closed httpx client')
    print('Closed httpserver')

asyncio.run(main())

This should produce an httpx.ReadTimeout error, and the httpserver should fail to exit. I've tested in on Linux, Python 3.9-3.12.

Thanks!

@csernazs
Copy link
Owner

Hi @TheTechromancer ,

Thanks!

I could reproduce your issue. Based on strace output it seems httpx behaves correcty by closing the connections, but I could not find why the reason the server is not stopping. The server is single-threaded so the shutdown is done in a way that a flag is set to True, then some event (or other synchronization primitive) is defined. Later on, once the server finished processing a request, it checks for this flag and notifies the waiting thread via this synchronization primitive. This means if the server is processing a request, it cannot handle the shutdown event. As httpx is seemingly closing the sockets correctly, this might be a bug in the server.

As a workaround you can now start the threaded server (a new feature in 1.0.11), in such case each request is served in a new thread, so stopping the server is much quicker and it is free of this mechanism.

So you can try replacing the server call to this:

with HTTPServer(threaded=True) as httpserver:

This worked for me. Could you also try this? This also performed much better, I think it is better to use this to serve requests made not sequentially (eg what your async code does).

Zsolt

@TheTechromancer
Copy link
Author

@csernazs thanks for the quick response! After some quick testing, that workaround seems to fix the issue. This bug has caused me so much pain, you have no idea how happy this makes me lol.

@csernazs
Copy link
Owner

Thanks! I'm happy that I could help :)

I think I'll do additional investigation on this later. pytest-httpserver uses werkzeug which then uses socketserver.py from stdlib, so as a maintainer for pytest-httpserver I rely on them to do the heavy lifting.

Also, interestingly:

  1. if you reduce the 100 to some lower value, the test pass
  2. If you add some sleep to here:
            for i in range(100):
                tasks.append(asyncio.create_task(make_request(client, httpserver.url_for("/"))))

             await asyncio.sleep(10) # <--- here

            print("Cancelling tasks")

then this also fixes it..

So there will be definitely a race condition..

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