Cohttp_lwt: body not consumed - leaking stream!

[mseri]

This often still happened to me, I found out that I had to drain the body every time it was unused: e.g in redirects

| 301 | 302 | 307 | 308 ->
      let* () = Cohttp_lwt.Body.drain_body body in

[rgrinberg]

We could add a finalizer to the request handler that drains the body, but note that this still encourages poor behavior. You should start reading the body as soon as it's possible to allow pipelining to proceed.

Another option is to change the callback type to something like:

val drain_body : Body.t -> read_body Lwt.t (* no-op if the user already drained the body. *)
type callback = Request.t * Body.t -> (Response.t * read_body) Lwt.t

[mseri]

The problem with the finalizer is that we cannot really call Lwt from it, otherwise we are forced to link unix in cohttp-lwt, at least we could not see how here: #696 (comment)

Calling the callback, maybe, would make things less ergonomic?

I think you make a good point that this fix would encourage bad behavior though. At least now we have a warning logged. Should I just add a paragraph about this in the readme then? What do you think?

[rgrinberg]

Wait, my bad. I initially thought you were referring to the problem on the server side. In which case we already use the finalizer appropriately. My suggestion above would improve the behavior further

On the client side, passing an explicit sink for the body is the safest bet.

(** Can be a GADT or just a fold *)
type 'a sink

val slurp : string sink

val discard : unit sink

val stream : (string -> unit Lwt.t) -> unit sink

val await : 'a sink -> 'a Lwt.t

val call : Uri.t -> Request.t -> 'a sink -> (Response.t * 'a Lwt.t) Lwt.t

In the above, I included await, but we call await already and just return its result.

[mseri]

Partly related issue: #578