mirror of
https://gitlab.isc.org/isc-projects/bind9
synced 2025-08-30 14:07:59 +00:00
DoH: Simplify http_do_bio()
This commit significantly simplifies the code flow in the http_do_bio() function, which is responsible for processing incoming and outgoing HTTP/2 data. It seems that the way it was structured before was indirectly caused by the presence of the missing callback calls bug, fixed in8b8f4d500d
. The change introduced by this commit is known to remove a bottleneck and allows reproducible and measurable performance improvement for long runs (>= 1h) of "stress:long:rpz:doh+udp:linux:*" tests. Additionally, it fixes a similar issue with potentially missing send callback calls processing and hardens the code against use-after-free errors related to the session object (they can potentially occur). (cherry picked from commit0956fb9b9e
)
This commit is contained in:
committed by
Artem Boldariev
parent
b2b68e2a18
commit
515d84e1f6
@@ -1293,7 +1293,10 @@ http_readcb(isc_nmhandle_t *handle ISC_ATTR_UNUSED, isc_result_t result,
|
||||
}
|
||||
isc_buffer_putmem(session->buf, region->base + readlen,
|
||||
unread_size);
|
||||
if (session->handle != NULL) {
|
||||
INSIST(VALID_NMHANDLE(session->handle));
|
||||
isc_nm_read_stop(session->handle);
|
||||
}
|
||||
http_do_bio_async(session);
|
||||
} else {
|
||||
/* We might have something to receive or send, do IO */
|
||||
@@ -1599,10 +1602,12 @@ http_too_many_active_streams(isc_nm_http_session_t *session) {
|
||||
static void
|
||||
http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
|
||||
isc_nm_cb_t send_cb, void *send_cbarg) {
|
||||
isc__nm_uvreq_t *req = NULL;
|
||||
size_t remaining = 0;
|
||||
REQUIRE(VALID_HTTP2_SESSION(session));
|
||||
|
||||
if (session->closed) {
|
||||
return;
|
||||
goto cancel;
|
||||
} else if (session->closing) {
|
||||
/*
|
||||
* There might be leftover callbacks waiting to be received
|
||||
@@ -1610,23 +1615,24 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
|
||||
if (session->sending == 0) {
|
||||
finish_http_session(session);
|
||||
}
|
||||
return;
|
||||
goto cancel;
|
||||
} else if (nghttp2_session_want_read(session->ngsession) == 0 &&
|
||||
nghttp2_session_want_write(session->ngsession) == 0 &&
|
||||
session->pending_write_data == NULL)
|
||||
{
|
||||
session->closing = true;
|
||||
if (session->handle != NULL) {
|
||||
isc_nm_read_stop(session->handle);
|
||||
}
|
||||
if (session->sending == 0) {
|
||||
finish_http_session(session);
|
||||
}
|
||||
goto cancel;
|
||||
}
|
||||
|
||||
if (send_cb != NULL) {
|
||||
INSIST(VALID_NMHANDLE(send_httphandle));
|
||||
http_send_outgoing(session, send_httphandle, send_cb,
|
||||
send_cbarg);
|
||||
return;
|
||||
}
|
||||
|
||||
INSIST(send_httphandle == NULL);
|
||||
INSIST(send_cb == NULL);
|
||||
INSIST(send_cbarg == NULL);
|
||||
|
||||
if (session->pending_write_data != NULL && session->sending == 0) {
|
||||
http_send_outgoing(session, NULL, NULL, NULL);
|
||||
return;
|
||||
else if (session->buf != NULL)
|
||||
{
|
||||
remaining = isc_buffer_remaininglength(session->buf);
|
||||
}
|
||||
|
||||
if (nghttp2_session_want_read(session->ngsession) != 0) {
|
||||
@@ -1635,9 +1641,7 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
|
||||
isc__nmsocket_timer_start(session->handle->sock);
|
||||
isc_nm_read(session->handle, http_readcb, session);
|
||||
session->reading = true;
|
||||
} else if (session->buf != NULL) {
|
||||
size_t remaining =
|
||||
isc_buffer_remaininglength(session->buf);
|
||||
} else if (session->buf != NULL && remaining > 0) {
|
||||
/* Leftover data in the buffer, use it */
|
||||
size_t remaining_after = 0;
|
||||
ssize_t readlen = 0;
|
||||
@@ -1661,8 +1665,12 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
|
||||
http_log_flooding_peer(session);
|
||||
failed_read_cb(ISC_R_RANGE, session);
|
||||
} else if ((size_t)readlen == remaining) {
|
||||
isc_buffer_free(&session->buf);
|
||||
http_do_bio(session, NULL, NULL, NULL);
|
||||
isc_buffer_clear(session->buf);
|
||||
isc_buffer_compact(session->buf);
|
||||
http_do_bio(session, send_httphandle, send_cb,
|
||||
send_cbarg);
|
||||
isc__nm_httpsession_detach(&tmpsess);
|
||||
return;
|
||||
} else if (remaining_after > 0 &&
|
||||
remaining_after < remaining)
|
||||
{
|
||||
@@ -1679,39 +1687,36 @@ http_do_bio(isc_nm_http_session_t *session, isc_nmhandle_t *send_httphandle,
|
||||
* it and that could overwhelm the server.
|
||||
*/
|
||||
http_do_bio_async(session);
|
||||
} else {
|
||||
http_send_outgoing(session, NULL, NULL, NULL);
|
||||
}
|
||||
|
||||
isc__nm_httpsession_detach(&tmpsess);
|
||||
return;
|
||||
} else {
|
||||
} else if (session->handle != NULL) {
|
||||
INSIST(VALID_NMHANDLE(session->handle));
|
||||
/*
|
||||
* Resume reading, it's idempotent, wait for more
|
||||
*/
|
||||
isc__nmsocket_timer_start(session->handle->sock);
|
||||
isc_nm_read(session->handle, http_readcb, session);
|
||||
}
|
||||
} else {
|
||||
} else if (session->handle != NULL) {
|
||||
INSIST(VALID_NMHANDLE(session->handle));
|
||||
/* We don't want more data, stop reading for now */
|
||||
isc_nm_read_stop(session->handle);
|
||||
}
|
||||
|
||||
/* we might have some data to send after processing */
|
||||
http_send_outgoing(session, NULL, NULL, NULL);
|
||||
|
||||
if (nghttp2_session_want_read(session->ngsession) == 0 &&
|
||||
nghttp2_session_want_write(session->ngsession) == 0 &&
|
||||
session->pending_write_data == NULL)
|
||||
{
|
||||
session->closing = true;
|
||||
isc_nm_read_stop(session->handle);
|
||||
if (session->sending == 0) {
|
||||
finish_http_session(session);
|
||||
}
|
||||
}
|
||||
http_send_outgoing(session, send_httphandle, send_cb, send_cbarg);
|
||||
|
||||
return;
|
||||
cancel:
|
||||
if (send_cb == NULL) {
|
||||
return;
|
||||
}
|
||||
req = isc__nm_uvreq_get(send_httphandle->sock);
|
||||
|
||||
req->cb.send = send_cb;
|
||||
req->cbarg = send_cbarg;
|
||||
isc_nmhandle_attach(send_httphandle, &req->handle);
|
||||
isc__nm_sendcb(send_httphandle->sock, req, ISC_R_CANCELED, true);
|
||||
}
|
||||
|
||||
static void
|
||||
|
Reference in New Issue
Block a user