From 950f828cd25454631232a81b5241680d4b7dadc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Fri, 3 Mar 2023 16:59:21 +0100 Subject: [PATCH 1/4] Offload the isc_http response processing to worker thread Prepare the statistics channel data in the offloaded worker thread, so the networking thread is not blocked by the process gathering data from various data structures. Only the netmgr send is then run on the networkin thread when all the data is already there. --- lib/isc/httpd.c | 47 +++++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 22 deletions(-) diff --git a/lib/isc/httpd.c b/lib/isc/httpd.c index 57cc0fd943..d2084ae180 100644 --- a/lib/isc/httpd.c +++ b/lib/isc/httpd.c @@ -598,6 +598,12 @@ isc__httpd_sendreq_new(isc_httpd_t *httpd) { isc_buffer_initnull(&req->bodybuffer); + /* + * We don't need to attach to httpd here because it gets only cleaned + * when the last handle has been detached + */ + req->httpd = httpd; + return (req); } @@ -749,10 +755,11 @@ httpd_compress(isc_httpd_sendreq_t *req) { #endif /* ifdef HAVE_ZLIB */ static void -prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd, - isc_httpd_sendreq_t **reqp) { - isc_httpd_sendreq_t *req = NULL; - isc_time_t now; +prepare_response(void *arg) { + isc_httpd_sendreq_t *req = arg; + isc_httpd_t *httpd = req->httpd; + isc_httpdmgr_t *mgr = httpd->mgr; + isc_time_t now = isc_time_now(); char datebuf[ISC_FORMATHTTPTIMESTAMP_SIZE]; const char *path = "/"; size_t path_len = 1; @@ -761,9 +768,8 @@ prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd, isc_result_t result; REQUIRE(VALID_HTTPD(httpd)); - REQUIRE(reqp != NULL && *reqp == NULL); + REQUIRE(req != NULL); - now = isc_time_now(); isc_time_formathttptimestamp(&now, datebuf, sizeof(datebuf)); if (httpd->up.field_set & (1 << ISC_UF_PATH)) { @@ -779,8 +785,6 @@ prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd, } UNLOCK(&mgr->lock); - req = isc__httpd_sendreq_new(httpd); - if (url == NULL) { result = mgr->render_404(httpd, NULL, NULL, &req->retcode, &req->retmsg, &req->mimetype, @@ -874,14 +878,20 @@ prepare_response(isc_httpdmgr_t *mgr, isc_httpd_t *httpd, } httpd->recvlen -= httpd->consume; httpd->consume = 0; +} + +static void +prepare_response_done(void *arg) { + isc_region_t r; + isc_httpd_sendreq_t *req = arg; + isc_httpd_t *httpd = req->httpd; /* - * We don't need to attach to httpd here because it gets only cleaned - * when the last handle has been detached + * Determine total response size. */ - req->httpd = httpd; + isc_buffer_usedregion(req->sendbuffer, &r); - *reqp = req; + isc_nm_send(httpd->handle, &r, httpd_senddone, req); } static void @@ -889,8 +899,6 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, isc_region_t *region, void *arg) { isc_httpd_t *httpd = arg; isc_httpdmgr_t *mgr = httpd->mgr; - isc_httpd_sendreq_t *req = NULL; - isc_region_t r; size_t last_len = 0; isc_result_t result; @@ -941,15 +949,10 @@ httpd_request(isc_nmhandle_t *handle, isc_result_t eresult, goto close_readhandle; } - prepare_response(mgr, httpd, &req); - - /* - * Determine total response size. - */ - isc_buffer_usedregion(req->sendbuffer, &r); - + isc_httpd_sendreq_t *req = isc__httpd_sendreq_new(httpd); isc_nmhandle_ref(handle); - isc_nm_send(handle, &r, httpd_senddone, req); + isc_work_enqueue(isc_loop(), prepare_response, prepare_response_done, + req); return; close_readhandle: From 23835c4afe3d1cd2704a6f68832a70ac128f6880 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 17 Apr 2024 19:58:16 +0200 Subject: [PATCH 2/4] Use xmlMemSetup() instead of xmlGcMemSetup() Since we don't have a specialized function for "atomic" allocations, it's better to just use xmlMemSetup() instead of xmlGcMemSetup() according to this: https://mail.gnome.org/archives/xml/2007-August/msg00032.html --- lib/isc/xml.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/lib/isc/xml.c b/lib/isc/xml.c index 9b5e52c818..bfea645ea5 100644 --- a/lib/isc/xml.c +++ b/lib/isc/xml.c @@ -53,9 +53,8 @@ isc__xml_initialize(void) { isc_mem_setname(isc__xml_mctx, "libxml2"); isc_mem_setdestroycheck(isc__xml_mctx, false); - RUNTIME_CHECK(xmlGcMemSetup(isc__xml_free, isc__xml_malloc, - isc__xml_malloc, isc__xml_realloc, - isc__xml_strdup) == 0); + RUNTIME_CHECK(xmlMemSetup(isc__xml_free, isc__xml_malloc, + isc__xml_realloc, isc__xml_strdup) == 0); xmlInitParser(); #endif /* HAVE_LIBXML2 */ From c7ed858c6e75b456e74a359bd235baed76df5185 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Wed, 17 Apr 2024 20:44:32 +0200 Subject: [PATCH 3/4] Supress the leak detection in xmlGetGlobalState The xmlGetGlobalState allocates per-thread memory that is not properly cleaned up when the libxml2 is used from offloaded threads. Add the function the the LeakSanitizer suppression list. --- suppr-lsan.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/suppr-lsan.txt b/suppr-lsan.txt index 050b86e9ae..da4984cd34 100644 --- a/suppr-lsan.txt +++ b/suppr-lsan.txt @@ -24,3 +24,4 @@ leak:pkcs11_enumerate_slots leak:pkcs11_getattr_alloc leak:pkcs11_init_key leak:pkcs11_strdup +leak:xmlGetGlobalState From fbea3bb255c54e9998c68777a360a58633a5c026 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Tue, 16 Apr 2024 17:37:30 +0200 Subject: [PATCH 4/4] Add CHANGES and release note for [GL #4680] --- CHANGES | 3 +++ doc/notes/notes-current.rst | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGES b/CHANGES index dcf18af4a0..67ad1f2265 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,6 @@ +6373. [func] Offload the isc_http response processing to worker + thread. [GL #4680] + 6372. [func] Implement signature jitter for dnssec-policy. [GL #4554] 6371. [bug] Access to the trust bytes in the ncache data needed to diff --git a/doc/notes/notes-current.rst b/doc/notes/notes-current.rst index 9812b77e46..cfbcc05c8d 100644 --- a/doc/notes/notes-current.rst +++ b/doc/notes/notes-current.rst @@ -33,7 +33,8 @@ Removed Features Feature Changes ~~~~~~~~~~~~~~~ -- None. +- Querying the statistics channel no longer blocks the DNS communication + on the networking event loop. :gl:`#4680` Bug Fixes ~~~~~~~~~