search

zio / zio-http

A next-generation Scala framework for building scalable, correct, and efficient HTTP clients and servers
https://zio.dev/zio-http
Apache License 2.0
785 stars 396 forks source link

Prevent OOM when receiving large request streams #3174

Open davidlar opened 3 days ago

davidlar commented 3 days ago

This PR aims to fix 3173 by making sure we don't read more data than we can handle. Now ctx.read() is called after the queue is drained by the consuming end of the stream. The call to ctx.read() will fill the queue with more data asynchronously. Nettys AUTO_READ flag is already turned off in zio-http and ctx.read should be called when we are ready for more data, not before.

I'm not sure if we should keep the unbounded queue or not. I guess it will only contain one chunk at most.

davidlar commented 2 days ago

Not sure I understand how ctx.read() can be called from AsyncBodyReader as it has be called from the consuming side of the queue. So it's probably to complex for me at least :-)

davidlar commented 2 days ago

I tried locally with queue.size.map(_ < 4096) and it can be a lot of data since the chunks in the queue can be big in some cases. Maybe we should count the queued bytes instead in a ref or something. Or lower that limit to 128 or so. What do you think @kyri-petrou ?

davidlar commented 2 days ago

It feels like the buffering before #3060 was meant to be maximum 4096 bytes, but was really max 4096 chunks which is not what we want, I guess... On the other hand I haven't seen smaller chunks than 8kb when testing this, so maybe queue.isEmpty is a good choice after all. I leave it like that if you want to approve! I will go away for the weekend...