mliezun / caddy-snake

Caddy plugin to serve Python apps
MIT License
87 stars 3 forks source link

Runtime error when streaming data #30

Closed alanpoulain closed 1 month ago

alanpoulain commented 3 months ago

When the response from a FastAPI application is streamed (using StreamingResponse), once the stream is done, the following error is raised:

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x20 pc=0x1595054]

goroutine 17 [running, locked to thread]:
github.com/mliezun/caddy-snake.asgi_receive_start(0x2, 0x719038989f30)
github.com/mliezun/caddy-snake@v0.0.7/caddysnake.go:706 +0xd4

Do you have an idea why?

alanpoulain commented 3 months ago

A simple reproducer:

from fastapi import FastAPI
from fastapi.responses import StreamingResponse

app = FastAPI()

def generator():
    for i in range(5):
        yield f"data: foo\n\n"

@app.get("/stream")
async def stream_test() -> StreamingResponse:
    return StreamingResponse(generator(), media_type='text/event-stream')

Test with:

import requests

response = requests.get('https://localhost/stream', verify=False, stream=True)
for chunk in response.iter_lines():
    print(f"Chunk: {chunk}")
mliezun commented 3 months ago

Oops, seems like a bug when reading/writing from the buffer in Go.

I'll take a look and send a fix as soon as possible.

alanpoulain commented 2 months ago

Please tell me if you need more information 🙂

mliezun commented 2 months ago

I've been able to reproduce this issue, but I haven't been able to track down the problem exactly. It looks like it's a race condition that makes Go read a value after it was already freed.

mliezun commented 2 months ago

I've opened a new PR #35 that fixes the problem. Please try it out and let me know if it works for you.

alanpoulain commented 2 months ago

Yes, no more error anymore! Thank you :slightly_smiling_face: The buffering is still there though (I will add a script to test in the related issue https://github.com/mliezun/caddy-snake/issues/31).

mliezun commented 1 month ago

Im closing this as completed because I just merged the PR that fixes this.