-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Fix crash when HTTP pipeline data arrives during async response writing #814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from 3 commits
35c78cc
4545e19
93e11a7
5084c45
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -111,17 +111,45 @@ bool HttpHandler::Init(int http_version) { | |
| void HttpHandler::Reset() { | ||
| state = WANT_RECV; | ||
| error = 0; | ||
| req->Reset(); | ||
| resp->Reset(); | ||
| // Create new request/response to avoid race condition with async handlers | ||
| // that may still hold shared_ptr references to the old req/resp objects. | ||
| // This prevents crashes when HTTP pipeline data arrives while an async | ||
| // response is still being written. | ||
| req = std::make_shared<HttpRequest>(); | ||
| resp = std::make_shared<HttpResponse>(); | ||
| if (protocol == HTTP_V2) { | ||
| resp->http_major = req->http_major = 2; | ||
| resp->http_minor = req->http_minor = 0; | ||
| } | ||
| ctx = NULL; | ||
| api_handler = NULL; | ||
| closeFile(); | ||
| if (writer) { | ||
| writer->Begin(); | ||
| writer->onwrite = NULL; | ||
| writer->onclose = NULL; | ||
| if (io) { | ||
| writer = std::make_shared<HttpResponseWriter>(io, resp); | ||
| writer->status = hv::SocketChannel::CONNECTED; | ||
| } else { | ||
| writer = NULL; | ||
| } | ||
| parser->InitRequest(req.get()); | ||
| // Re-hook http_cb for the new request object | ||
| req->http_cb = [this](HttpMessage* msg, http_parser_state state, const char* data, size_t size) { | ||
| if (this->state == WANT_CLOSE) return; | ||
| switch (state) { | ||
| case HP_HEADERS_COMPLETE: | ||
| if (this->error != 0) return; | ||
| onHeadersComplete(); | ||
| break; | ||
| case HP_BODY: | ||
| if (this->error != 0) return; | ||
| onBody(data, size); | ||
| break; | ||
| case HP_MESSAGE_COMPLETE: | ||
| onMessageComplete(); | ||
| break; | ||
| default: | ||
| break; | ||
| } | ||
| }; | ||
|
||
| } | ||
|
|
||
| void HttpHandler::Close() { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reset() now replaces
writerwith a newHttpResponseWriterbound to the samehio_t. The old writer will be destroyed as soon as its last shared_ptr is released, andhv::Channel::~Channel()closes the underlyinghio_twhenisOpened()is true—this will break keep-alive (old writer destroyed immediately on normal Reset) and can also close an active connection while a subsequent request is being processed. Additionally, constructing a new Channel/SocketChannel on the samehio_toverwriteshio_context, so on_write/on_close callbacks will no longer be routed to the in-flight writer instance. Consider keeping a single Channel wrapper per connection and making per-request writers non-owning (no hio_context mutation / no close-on-destroy), or otherwise ensure retiring writers cannot close or steal callbacks from the connection while still allowing the active writer to finish sending.