-
-
Notifications
You must be signed in to change notification settings - Fork 92
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
1.2.0
doesn't shut down properly
#252
Comments
@tim-habitat can you please provide more details on this (eg: OS/container, Python version, libraries used, granian cmd used) and possibly logs/MRE validating your statement? |
sorry yes, i'll try to get a reproducible example here in the next few days! note: i did have a lifespan to create database tables |
similar issue here with Django running granian as asgi server:
running in docker on mac m2 |
I'm experiencing similar issues when trying to shutdown granian. It is stuck on stopping the worker:
To stop it I have to kill Granian via SIGKILL. System:
|
@RafaelWO which OS? Linux? |
@gi0baro Yes! I updated my comment. I downgraded to |
It is a weird issue and hard to reproduce... What I've found so far is that if I disable my custom HTTP middleware I was not able to run into the shutdown bug. (I'm using FastAPI and adding a function via |
That sounds quite strange, I agree. Is that still only on 1.2 or also 1.1? |
I might have found a somewhat minimal reproducible example:
# file: main.py
from fastapi import FastAPI, Request
app = FastAPI()
# If I comment/remove this middleware, everything works fine
@app.middleware("http")
async def log_req(request: Request, call_next):
response = await call_next(request)
print("[MIDDLEWARE]", request.method, request.url, response.status_code)
return response
data = [{"id": 1, "type": "foo"}, {"id": 2, "type": "bar"}]
@app.get("/items")
def read_items():
print("Hello!")
return data To reproduce:
I can reproduce this issue at least 90% of the time 😆 |
LoL. Super weird. |
You are right! If the route is an P.S. Could you also reproduce this issue with my code above? |
I'm not sure, it might be related to the way workers handle signals, but further investigations are needed.
Not yet, I might have some time during the weekend to run tests. |
Bisecting shows that this issue is due to #232, specifically the changes to src/callbacks.rs and src/runtime.rs. Something is causing certain requests to have hanging tasks, which prevents threads from shutting down. Incidentally this may also be the cause of #276, because those hanging tasks are preventing some requests from being cleaned up. |
Can you elaborate? |
Yes, asyncio tasks on the event loop, and the main thread running the Python event loop in the worker. The issue appears to be related to cancellations - #279 fixes it in some cases for me, although there seems to be something else that is still broken. |
FWIW, the shutdown issue I see when running on our larger codebase also happens on 1.1.2. |
Then it is not related to changes made in #232: 1.1.x still uses the standard PyO3 |
The changes from #232 do appear related, because the repro provided fails consistently, but it seems very tricky to figure out 😢 |
Fixes it for me at least, thanks @gi0baro! |
Closing this due to changes in 1.3. |
I also tested it again with granian 1.3 and it's fixed! Thx @gi0baro 🙂 |
Hi @gi0baro 👋 I have a project with Django Tenants and Django Channels. I tried granian, and made a very dummy frontend with WebSocket. Django Channels on connection sent this to frontend: await self.send(
text_data=json.dumps(
{"type": "connection_established", "message": "You are now connected!"}
)
) It worked, but after some browser refreshes, I stopped seeing WebSocket information on the granian, and after a couple of minutes it suddenly picked a bunch of those previous requests but never picking the current one. Closing the server and opening again was not solving, only by restarting the computer. 😅 Not sure if it is not closing properly and then starts to throttle, no idea. Let me know what can I do to help you 🙏 |
@Tragio the issue you reported seems to be unrelated to the OP. |
When upgrading to
v1.2.0
it seems the shutdown forasgi
doesn't work properly - it seems to be hanging.The text was updated successfully, but these errors were encountered: