I've never actually tried this myself, but does Lwt.cancel not do what you want?

Haven't looked at your code however yet.

Same question as @rgrinberg: the convention in Lwt is to raise Lwt.Canceled which ripples up through the cancelled threads until caught. Is there a case where Lwt.cancel fails to cleanly shut down a Cohttp thread? It's quite likely that something could go wrong since this is a very untested path (in most cooperatively threaded code sadly, not just Cohttp/Lwt)

Okay, I've done more testings. The problem with Lwt.cancel is that it indeed cancels the thread but none of the handlers for on_terminate are being called, which will result in fd leaks and all sort of confusions.. Will see if I can do another, cleaner pass with this in mind..

You're probably aware of this, but perhaps Lwt.on_cancel can be make sure cleanup occurs.

Which reminds me that we need to make cleanup exception safe as well.

Yes but for this to work, you need to be able to cancel a thread below the body stream. As far as I can see, canceling my processing thread does not propagate back to the stream, thus the stream's fds aren't cleanup either..

I'm talking about this piece of code: https://github.com/mirage/ocaml-cohttp/blob/master/lwt-core/cohttp_lwt.ml#L69
I'm aware that the Gc should do the job eventually but I've never been quite keen on leaving fd close up to the Gc..

Good point. The interface absolutely should not depend on the Gc cleaning up the file descriptors.

I'm think I'm running into this as well, with a Cohttp server ending up with thousands of ESTABLISHED connections to nowhere. I'll try to come up with a minimal, reproducible test case next week.

A small repro case would be appreciated. The GC shouldn't have to clean up fds to prevent a leak indeed.

I removed my previous comments because I'm not sure the problem I'm seeing is related.

I have an example here which seems to never free client connections:
https://github.com/hcarty/ocaml-cohttp/blob/lwt-fd-leak/bin/cohttp_lwt_leak.ml

and a potential fix/patch here:
hcarty@c4136c4

If the patch looks appropriate I'll submit a pull request.

With this patch an internal system of ours which was previously accumulating 10k+ connections/day is now running without any leaked connections.

This has become out of date, feel free to send a new PR if needed.