From e850946557469f2cbe4fab76d1c52227ddf81a93 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Micha=C5=82=20K=C4=99pie=C5=84?= <michal@isc.org>
Date: Thu, 21 Apr 2022 14:19:39 +0200
Subject: [PATCH] Prevent memory bloat caused by a jemalloc quirk

Since version 5.0.0, decay-based purging is the only available dirty
page cleanup mechanism in jemalloc.  It relies on so-called tickers,
which are simple data structures used for ensuring that certain actions
are taken "once every N times".  Ticker data (state) is stored in a
thread-specific data structure called tsd in jemalloc parlance.  Ticks
are triggered when extents are allocated and deallocated.  Once every
1000 ticks, jemalloc attempts to release some of the dirty pages hanging
around (if any).  This allows memory use to be kept in check over time.

This dirty page cleanup mechanism has a quirk.  If the first
allocator-related action for a given thread is a free(), a
minimally-initialized tsd is set up which does not include ticker data.
When that thread subsequently calls *alloc(), the tsd transitions to its
nominal state, but due to a certain flag being set during minimal tsd
initialization, ticker data remains unallocated.  This prevents
decay-based dirty page purging from working, effectively enabling memory
exhaustion over time. [1]

The quirk described above has been addressed (by moving ticker state to
a different structure) in jemalloc's development branch [2], but not in
any numbered jemalloc version released to date (the latest one being
5.2.1 as of this writing).

Work around the problem by ensuring that every thread spawned by
isc_thread_create() starts with a malloc() call.  Avoid immediately
calling free() for the dummy allocation to prevent an optimizing
compiler from stripping away the malloc() + free() pair altogether.

An alternative implementation of this workaround was considered that
used a pair of isc_mem_create() + isc_mem_destroy() calls instead of
malloc() + free(), enabling the change to be fully contained within
isc__trampoline_run() (i.e. to not touch struct isc__trampoline), as the
compiler is not allowed to strip away arbitrary function calls.
However, that solution was eventually dismissed as it triggered
ThreadSanitizer reports when tools like dig, nsupdate, or rndc exited
abruptly without waiting for all worker threads to finish their work.

[1] https://github.com/jemalloc/jemalloc/issues/2251
[2] https://github.com/jemalloc/jemalloc/commit/c259323ab3082324100c708109dbfff660d0f4b8

(cherry picked from commit 7aa7b6474bc5ea2b4ec4806c7509dc5ea73396e1)
Conflict: NA
Reference: https://gitlab.isc.org/isc-projects/bind9/-/commit/e850946557469f2cbe4fab76d1c52227ddf81a93
---
 lib/isc/trampoline.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/lib/isc/trampoline.c b/lib/isc/trampoline.c
index 4caa2e7574..e133c40084 100644
--- a/lib/isc/trampoline.c
+++ b/lib/isc/trampoline.c
@@ -31,6 +31,7 @@ struct isc__trampoline {
 	uintptr_t self;
 	isc_threadfunc_t start;
 	isc_threadarg_t arg;
+	void *jemalloc_enforce_init;
 };
 
 static isc_once_t isc__trampoline_initialize_once = ISC_ONCE_INIT;
@@ -170,6 +171,7 @@ isc__trampoline_detach(isc__trampoline_t *trampoline) {
 		isc__trampoline_min = trampoline->tid;
 	}
 
+	free(trampoline->jemalloc_enforce_init);
 	free(trampoline);
 
 	UNLOCK(&isc__trampoline_lock);
@@ -185,6 +187,15 @@ isc__trampoline_attach(isc__trampoline_t *trampoline) {
 	/* Initialize the trampoline */
 	isc_tid_v = trampoline->tid;
 	trampoline->self = isc_thread_self();
+
+	/*
+	 * Ensure every thread starts with a malloc() call to prevent memory
+	 * bloat caused by a jemalloc quirk.  While this dummy allocation is
+	 * not used for anything, free() must not be immediately called for it
+	 * so that an optimizing compiler does not strip away such a pair of
+	 * malloc() + free() calls altogether, as it would foil the fix.
+	 */
+	trampoline->jemalloc_enforce_init = malloc(8);
 }
 
 isc_threadresult_t
-- 
2.23.0