closing fd's is sometimes helpful #167
Conversation
We don't have connection pooling for the client anyway so at least don't be so buggy and close fd's after every request.
| @@ -155,6 +155,10 @@ module Client = struct | |||
| | `Ok res -> | |||
| (* Build a response pipe for the body *) | |||
| let rd = pipe_of_body (Response.read_body_chunk res) ic oc in | |||
| don't_wait_for ( | |||
There was a problem hiding this comment.
why not wait for these to close? It should be pretty fast, and anything else seems race-prone if the close isn't the right thing to do for some reason.
There was a problem hiding this comment.
Because we don't how long it will take the pipe to finish reading. For example, what if we have some huge body and the pipe will stall with push back until the client has read something.
There was a problem hiding this comment.
Oh of course -- sorry, I missed the vital Pipe.closed >>= on my first read. In my defence, it's because I'm busy hacking on the Conduit patches to let a release be cut ;-)
There was a problem hiding this comment.
NP ^_^. Are we having a release shortly after?
There was a problem hiding this comment.
Yep! I never intended for trunk to accumulate patches this long. Conduit is necessary for Mirage HTTP(S) requests to work though, so lots of combinations to test (and a DNS resolver needs to be hooked in too, which is ongoing in mirage/ocaml-dns)
|
This makes @pdonadeo's example work longer than I care to test it at this time. |
|
merged in 1bf96f8 |
Re-enable md2mld
We don't have connection pooling for the client anyway so at least don't
be so buggy and close fd's after every request.