encode / starlette

The little ASGI framework that shines. 🌟
https://www.starlette.io/
BSD 3-Clause "New" or "Revised" License
10.26k stars 932 forks source link

Contextvars are not reset between requests if the request contains multipart form data #2335

Open Kludex opened 12 months ago

Kludex commented 12 months ago

Discussed in https://github.com/encode/starlette/discussions/2312

Originally posted by **simon-sk** October 26, 2023 Hi Starlette team, we are experiencing an issue with Starlette where `contextvars` set in a middleware are leaking between requests if the request contains multi-part form data and the client session is reused. I constructed a minimal example which sets a context var in a middleware and resets it after the app has been called. It also raises an exception if the var is already set: ```python import contextvars from starlette.applications import Starlette from starlette.middleware import Middleware from starlette.responses import JSONResponse from starlette.routing import Route var = contextvars.ContextVar("var", default=None) class ASGIMiddleware: def __init__(self, app): self.app = app async def __call__(self, scope, receive, send): if scope["type"] != "http": return await self.app(scope, receive, send) if var.get() is not None: raise ValueError(f"var is already set {var.get}") token = var.set("TEST") await self.app(scope, receive, send) var.reset(token) async def route(request): return JSONResponse({"hello": "world"}) app = Starlette( middleware=[Middleware(ASGIMiddleware)], routes=[Route('/', route, methods=["POST"])] ) if __name__ == "__main__": import uvicorn uvicorn.run( "app.main_starlette:app", host="0.0.0.0", port=8001, ) ``` Then, I execute a second script which calls this endpoint using a `httpx` client and includes a files object: ```python import httpx if __name__ == "__main__": url = "http://localhost:8001/" files = {'file': open('./test.yaml', 'rb')} with httpx.Client() as client: for i in range(2): r = client.post(url, timeout=None, files=files) assert r.status_code == 200 ``` This results in a server error due to the fact that the variable is already set (sometimes it is necessary to run the client script multiple times as the issue does not always occur). If I execute the same script without multipart data, it works without issues: ```python ... r = client.post(url, timeout=None) ... ``` We have also experienced this issue without sending multipart form data for requests within the same client session but it occurs significantly less often and unfortunately I am not able to reproduce it consistently. We experienced this issue with Python 3.9 and 3.10, Starlette v0.31.1, and uvicorn v0.23.2.
Kludex commented 10 months ago

I can't reproduce on the latest versions of the packages, and considering Python 3.9 and 3.10.

If you can create a repository, and show me in the CI how this fails, I can continue helping.

simon-sk commented 9 months ago

@Kludex I created a repo here that reproduces the issue in this action run.

The issue does not always occur on the first request, so I added a loop that sends multiple requests in a row.

Kludex commented 3 months ago

@Kludex I created a repo here that reproduces the issue in this action run.

The issue does not always occur on the first request, so I added a loop that sends multiple requests in a row.

I can reproduce your MRE using uvicorn running with the AsyncIO event loop on Python 3.9. I cannot reproduce with the latest Python version, and I also can't reproduce with uvloop.

9dogs commented 3 weeks ago

I'm having the same issue with Python 3.11.9 and starlette==0.38.6 (transient dep from the FastAPI==0.115.0).

In my case, I'm doing simple JSON POST, but I only notice it on "high enough" loads. Installing the uvloop fixes the issue indeed...

Pol-Zeimet commented 3 days ago

I can confirm this bug. I have to work with some older versions due to some dependencies. With starlette== 0.23.1 and fastapi==0.90.1 and python 3.11 I can reproduce the error. Can also confirm that uvloop fixed it. Simply installing it in the venv did the trick, funnily enough. I didnt even have to change any code.