The evhttp code currently lacks a couple things to properly use chunked replies:
- no way to know that a connection failed (client closed the connection)
- no way to know that the current chunk has been sent
The first issue leads to crashes and the second leads to high memory usage as you end up buffering your chunks in memory.
Proposed patch attached, this code is tested and included in my "forked-daapd" project. Works like a charm.
Proposed patch, adding callbacks to evhttp for chunked replies
Hi Julien,
I have been terribly slow at look at these; the failure scenario is already handled by evhttp_connection_set_closecb(); there is also a new accessor evhttp_request_get_connection() to get the connection object from the request.
I also submitted a fix so that the http server no longer crashes when the client closes the connection; this is orthogonal to getting the close cb.
I am going to look into the chunk completion callback and see if there is a way to fix this without introducing a new send_chunk function.
Hi Niels!
Better late than never, I'm glad to see there's some interest in those patches :)
As I understand it, this is all libevent2 material, right?
For the fail_cb, you're absolutely right that evhttp_connection's closecb does the job. I don't know why I did not use it; probably because evhttp_connection is an opaque type to the evhttp user and I did not want to mess with it. I've migrated my code to using that callback and removed my extension and it all works fine, as expected.
For the chunk completion callback, maybe this could become a new completion callback on the request. For chunked replies it would be called after each chunk is sent and otherwise it would be called after the reply has been sent; the point is, it may have some use outside chunked replies, too.
While we're here, I think there's a typo in the comment modified in http.h in commit 39781801; I think it should read "user" instead of "server" and you should explicitly note that it applies to the case where get_connection() returns NULL.
JB.
Moving to patch tracker.