Enforce linking with jemalloc

Add OPENSSL_cleanup to tls_shutdown function (9.16)

See merge request isc-projects/bind9!5625

CHANGES

@@ -1,3 +1,7 @@
+5776.	[bug]		Add a missing isc_condition_destroy() for nmsocket
+			condition variable and add missing isc_mutex_destroy()
+			for nmworker lock. [GL #3051]
+
 5773.	[func]		Change the message when accepting TCP connection has
 			failed to say "Accepting TCP connection failed" and
 			change the log level for ISC_R_NOTCONNECTED, ISC_R_QUOTA

config.h.in

@@ -288,6 +288,9 @@
 /* Define to 1 if you have the <net/route.h> header file. */
 #undef HAVE_NET_ROUTE_H
 
+/* Define to 1 if you have the `OPENSSL_cleanup' function. */
+#undef HAVE_OPENSSL_CLEANUP
+
 /* define if OpenSSL supports Ed25519 */
 #undef HAVE_OPENSSL_ED25519
 

configure

@@ -780,6 +780,8 @@ PANDOC
 W3M
 LN
 ARFLAGS
+JEMALLOC_LIBS
+JEMALLOC_CFLAGS
 XTARGETS
 PKG_CONFIG_LIBDIR
 PKG_CONFIG_PATH
@@ -967,6 +969,8 @@ LT_SYS_LIBRARY_PATH
 PKG_CONFIG
 PKG_CONFIG_PATH
 PKG_CONFIG_LIBDIR
+JEMALLOC_CFLAGS
+JEMALLOC_LIBS
 PYTHON
 MAXMINDDB_CFLAGS
 MAXMINDDB_LIBS
@@ -1687,7 +1691,7 @@ Optional Packages:
   --with-libidn2=PATH     enable IDN support using GNU libidn2
                           [yes|no(default)|path]
   --with-cmocka=detect    enable CMocka based tests (default is detect)
-  --with-tuning=ARG       Specify server tuning (default or small)
+  --with-tuning=ARG       Specify server tuning (default or large)
   --with-dlopen=ARG       support dynamically loadable DLZ and DYNDB drivers
   --with-dnsrps-libname   DNSRPS provider library name (librpz.so)
   --with-dnsrps-dir       path to DNSRPS provider library
@@ -1725,6 +1729,10 @@ Some influential environment variables:
               directories to add to pkg-config's search path
   PKG_CONFIG_LIBDIR
               path overriding pkg-config's built-in search path
+  JEMALLOC_CFLAGS
+              C compiler flags for JEMALLOC, overriding pkg-config
+  JEMALLOC_LIBS
+              linker flags for JEMALLOC, overriding pkg-config
   PYTHON      path to python executable
   MAXMINDDB_CFLAGS
               C compiler flags for MAXMINDDB, overriding pkg-config
@@ -12321,6 +12329,104 @@ else
 fi
 
 
+# Enforce jemalloc
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for jemalloc" >&5
+$as_echo_n "checking for jemalloc... " >&6; }
+
+pkg_failed=no
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for jemalloc" >&5
+$as_echo_n "checking for jemalloc... " >&6; }
+
+if test -n "$JEMALLOC_CFLAGS"; then
+    pkg_cv_JEMALLOC_CFLAGS="$JEMALLOC_CFLAGS"
+ elif test -n "$PKG_CONFIG"; then
+    if test -n "$PKG_CONFIG" && \
+    { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"jemalloc\""; } >&5
+  ($PKG_CONFIG --exists --print-errors "jemalloc") 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; then
+  pkg_cv_JEMALLOC_CFLAGS=`$PKG_CONFIG --cflags "jemalloc" 2>/dev/null`
+		      test "x$?" != "x0" && pkg_failed=yes
+else
+  pkg_failed=yes
+fi
+ else
+    pkg_failed=untried
+fi
+if test -n "$JEMALLOC_LIBS"; then
+    pkg_cv_JEMALLOC_LIBS="$JEMALLOC_LIBS"
+ elif test -n "$PKG_CONFIG"; then
+    if test -n "$PKG_CONFIG" && \
+    { { $as_echo "$as_me:${as_lineno-$LINENO}: \$PKG_CONFIG --exists --print-errors \"jemalloc\""; } >&5
+  ($PKG_CONFIG --exists --print-errors "jemalloc") 2>&5
+  ac_status=$?
+  $as_echo "$as_me:${as_lineno-$LINENO}: \$? = $ac_status" >&5
+  test $ac_status = 0; }; then
+  pkg_cv_JEMALLOC_LIBS=`$PKG_CONFIG --libs "jemalloc" 2>/dev/null`
+		      test "x$?" != "x0" && pkg_failed=yes
+else
+  pkg_failed=yes
+fi
+ else
+    pkg_failed=untried
+fi
+
+
+
+if test $pkg_failed = yes; then
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+
+if $PKG_CONFIG --atleast-pkgconfig-version 0.20; then
+        _pkg_short_errors_supported=yes
+else
+        _pkg_short_errors_supported=no
+fi
+        if test $_pkg_short_errors_supported = yes; then
+	        JEMALLOC_PKG_ERRORS=`$PKG_CONFIG --short-errors --print-errors --cflags --libs "jemalloc" 2>&1`
+        else
+	        JEMALLOC_PKG_ERRORS=`$PKG_CONFIG --print-errors --cflags --libs "jemalloc" 2>&1`
+        fi
+	# Put the nasty error message in config.log where it belongs
+	echo "$JEMALLOC_PKG_ERRORS" >&5
+
+	as_fn_error $? "Package requirements (jemalloc) were not met:
+
+$JEMALLOC_PKG_ERRORS
+
+Consider adjusting the PKG_CONFIG_PATH environment variable if you
+installed software in a non-standard prefix.
+
+Alternatively, you may set the environment variables JEMALLOC_CFLAGS
+and JEMALLOC_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details." "$LINENO" 5
+elif test $pkg_failed = untried; then
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: no" >&5
+$as_echo "no" >&6; }
+	{ { $as_echo "$as_me:${as_lineno-$LINENO}: error: in \`$ac_pwd':" >&5
+$as_echo "$as_me: error: in \`$ac_pwd':" >&2;}
+as_fn_error $? "The pkg-config script could not be found or is too old.  Make sure it
+is in your PATH or set the PKG_CONFIG environment variable to the full
+path to pkg-config.
+
+Alternatively, you may set the environment variables JEMALLOC_CFLAGS
+and JEMALLOC_LIBS to avoid the need to call pkg-config.
+See the pkg-config man page for more details.
+
+To get pkg-config, see <http://pkg-config.freedesktop.org/>.
+See \`config.log' for more details" "$LINENO" 5; }
+else
+	JEMALLOC_CFLAGS=$pkg_cv_JEMALLOC_CFLAGS
+	JEMALLOC_LIBS=$pkg_cv_JEMALLOC_LIBS
+        { $as_echo "$as_me:${as_lineno-$LINENO}: result: yes" >&5
+$as_echo "yes" >&6; }
+
+fi
+CFLAGS="$JEMALLOC_CFLAGS $CFLAGS"
+LDFLAGS="$JEMALLOC_LDFLAGS $LDFLAGS"
+LIBS="$JEMALLOC_LIBS $LIBS"
+
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking whether to enable fuzzing mode" >&5
 $as_echo_n "checking whether to enable fuzzing mode... " >&6; }
 case $enable_fuzzing in #(
@@ -22310,19 +22416,19 @@ done
 if test "${with_tuning+set}" = set; then :
   withval=$with_tuning;
 else
-  with_tuning=no
+  with_tuning=small
 fi
 
 
 case $with_tuning in #(
-  small) :
-    { $as_echo "$as_me:${as_lineno-$LINENO}: using small system tuning" >&5
-$as_echo "$as_me: using small system tuning" >&6;} ;; #(
-  *) :
+  large) :
 
 $as_echo "#define TUNE_LARGE 1" >>confdefs.h
 
-	 { $as_echo "$as_me:${as_lineno-$LINENO}: using default system tuning" >&5
+		 { $as_echo "$as_me:${as_lineno-$LINENO}: using small system tuning" >&5
+$as_echo "$as_me: using small system tuning" >&6;} ;; #(
+  *) :
+    { $as_echo "$as_me:${as_lineno-$LINENO}: using default system tuning" >&5
 $as_echo "$as_me: using default system tuning" >&6;} ;;
 esac
 

configure.ac

@@ -117,6 +117,13 @@ AC_ARG_ENABLE([fuzzing],
 	      [],
 	      [enable_fuzzing=no])
 
+# Enforce jemalloc
+AC_MSG_CHECKING([for jemalloc])
+PKG_CHECK_MODULES([JEMALLOC], [jemalloc])
+CFLAGS="$JEMALLOC_CFLAGS $CFLAGS"
+LDFLAGS="$JEMALLOC_LDFLAGS $LDFLAGS"
+LIBS="$JEMALLOC_LIBS $LIBS"
+
 AC_MSG_CHECKING([whether to enable fuzzing mode])
 AS_CASE([$enable_fuzzing],
 	[no],[AC_MSG_RESULT([no])],
@@ -832,7 +839,7 @@ AC_COMPILE_IFELSE(
 # Check for functions added in OpenSSL or LibreSSL
 #
 
-AC_CHECK_FUNCS([OPENSSL_init_ssl OPENSSL_init_crypto])
+AC_CHECK_FUNCS([OPENSSL_init_ssl OPENSSL_init_crypto OPENSSL_cleanup])
 AC_CHECK_FUNCS([CRYPTO_zalloc])
 AC_CHECK_FUNCS([EVP_CIPHER_CTX_new EVP_CIPHER_CTX_free])
 AC_CHECK_FUNCS([EVP_MD_CTX_new EVP_MD_CTX_free EVP_MD_CTX_reset])
@@ -2299,13 +2306,13 @@ AC_CHECK_FUNCS(setlocale)
 # [pairwise: --with-tuning=small, --without-tuning]
 AC_ARG_WITH([tuning],
 	    AS_HELP_STRING([--with-tuning=ARG],
-			   [Specify server tuning (default or small)]),
-	    [],[with_tuning=no])
+			   [Specify server tuning (default or large)]),
+	    [],[with_tuning=small])
 
 AS_CASE([$with_tuning],
-	[small],[AC_MSG_NOTICE(using small system tuning)],
-	[AC_DEFINE(TUNE_LARGE, 1, [Define to use default system tuning.])
-	 AC_MSG_NOTICE(using default system tuning)])
+	[large],[AC_DEFINE(TUNE_LARGE, 1, [Define to use default system tuning.])
+		 AC_MSG_NOTICE(using small system tuning)],
+	[AC_MSG_NOTICE(using default system tuning)])
 
 #
 # was --enable-querytrace specified?

doc/notes/notes-current.rst

@@ -52,3 +52,6 @@ Bug Fixes
   ``rndc reconfig``, then bringing back the removed ``catalog-zone`` clause and
   running ``rndc reconfig`` again caused ``named`` to crash. This has been fixed.
   :gl:`#1608`
+
+- On FreeBSD, a TCP connection would leak a small amount of heap memory leading
+  to out-of-memory problem in a long run. This has been fixed. :gl:`#3051`

lib/isc/hp.c

@@ -45,6 +45,7 @@
 
 #include <inttypes.h>
 
+#include <isc/align.h>
 #include <isc/atomic.h>
 #include <isc/hp.h>
 #include <isc/mem.h>
@@ -53,14 +54,12 @@
 #include <isc/thread.h>
 #include <isc/util.h>
 
-#define HP_MAX_THREADS 128
-static int isc__hp_max_threads = HP_MAX_THREADS;
-#define HP_MAX_HPS     4 /* This is named 'K' in the HP paper */
-#define CLPAD	       (128 / sizeof(uintptr_t))
-#define HP_THRESHOLD_R 0 /* This is named 'R' in the HP paper */
+#define CACHELINE_SIZE 64
 
-/* Maximum number of retired objects per thread */
-static int isc__hp_max_retired = HP_MAX_THREADS * HP_MAX_HPS;
+static int isc__hp_max_threads = 1;
+#define HP_MAX_HPS     4 /* This is named 'K' in the HP paper */
+#define CLPAD	       (CACHELINE_SIZE / sizeof(uintptr_t))
+#define HP_THRESHOLD_R 0 /* This is named 'R' in the HP paper */
 
 typedef struct retirelist {
 	int size;
@@ -69,10 +68,11 @@ typedef struct retirelist {
 
 struct isc_hp {
 	int max_hps;
+	int max_retired;
 	isc_mem_t *mctx;
-	atomic_uintptr_t **hp;
-	retirelist_t **rl;
 	isc_hp_deletefunc_t *deletefunc;
+	alignas(CACHELINE_SIZE) atomic_uintptr_t **hp;
+	alignas(CACHELINE_SIZE) retirelist_t **rl;
 };
 
 static inline int
@@ -82,35 +82,72 @@ tid(void) {
 
 void
 isc_hp_init(int max_threads) {
+	REQUIRE(max_threads > 0);
+
+	if (isc__hp_max_threads > max_threads) {
+		return;
+	}
+
 	isc__hp_max_threads = max_threads;
-	isc__hp_max_retired = max_threads * HP_MAX_HPS;
+}
+
+static size_t
+hp_clpad(size_t max_hps) {
+	size_t hp_size = max_hps * sizeof(atomic_uintptr_t);
+	size_t hp_padding = 0;
+	while (hp_size > CACHELINE_SIZE) {
+		hp_size -= CACHELINE_SIZE;
+	}
+	if (hp_size > 0) {
+		hp_padding = CACHELINE_SIZE / hp_size;
+	}
+
+	return (hp_size + hp_padding);
 }
 
 isc_hp_t *
 isc_hp_new(isc_mem_t *mctx, size_t max_hps, isc_hp_deletefunc_t *deletefunc) {
 	isc_hp_t *hp = isc_mem_get(mctx, sizeof(*hp));
 
+	REQUIRE(isc__hp_max_threads > 0);
+	REQUIRE(max_hps <= HP_MAX_HPS);
+
 	if (max_hps == 0) {
 		max_hps = HP_MAX_HPS;
 	}
 
-	*hp = (isc_hp_t){ .max_hps = max_hps, .deletefunc = deletefunc };
+	*hp = (isc_hp_t){
+		.max_hps = max_hps,
+		.max_retired = isc__hp_max_threads * max_hps,
+		.deletefunc = deletefunc,
+	};
 
 	isc_mem_attach(mctx, &hp->mctx);
 
 	hp->hp = isc_mem_get(mctx, isc__hp_max_threads * sizeof(hp->hp[0]));
-	hp->rl = isc_mem_get(mctx, isc__hp_max_threads * sizeof(hp->rl[0]));
-
 	for (int i = 0; i < isc__hp_max_threads; i++) {
-		hp->hp[i] = isc_mem_get(mctx, CLPAD * 2 * sizeof(hp->hp[i][0]));
-		hp->rl[i] = isc_mem_get(mctx, sizeof(*hp->rl[0]));
-		*hp->rl[i] = (retirelist_t){ .size = 0 };
-
+		hp->hp[i] = isc_mem_get(mctx, hp_clpad(hp->max_hps));
 		for (int j = 0; j < hp->max_hps; j++) {
 			atomic_init(&hp->hp[i][j], 0);
 		}
-		hp->rl[i]->list = isc_mem_get(
-			hp->mctx, isc__hp_max_retired * sizeof(uintptr_t));
+	}
+
+	/*
+	 * It's not nice that we have a lot of empty space, but we need padding
+	 * to avoid false sharing.
+	 */
+	hp->rl = isc_mem_get(mctx,
+			     (isc__hp_max_threads * CLPAD) * sizeof(hp->rl[0]));
+
+	for (int i = 0; i < isc__hp_max_threads; i++) {
+		retirelist_t *rl;
+
+		rl = isc_mem_get(mctx, sizeof(*rl));
+		rl->size = 0;
+		rl->list = isc_mem_get(hp->mctx,
+				       hp->max_retired * sizeof(uintptr_t));
+
+		hp->rl[i * CLPAD] = rl;
 	}
 
 	return (hp);
@@ -119,19 +156,22 @@ isc_hp_new(isc_mem_t *mctx, size_t max_hps, isc_hp_deletefunc_t *deletefunc) {
 void
 isc_hp_destroy(isc_hp_t *hp) {
 	for (int i = 0; i < isc__hp_max_threads; i++) {
-		isc_mem_put(hp->mctx, hp->hp[i],
-			    CLPAD * 2 * sizeof(hp->hp[i][0]));
+		retirelist_t *rl = hp->rl[i * CLPAD];
 
-		for (int j = 0; j < hp->rl[i]->size; j++) {
-			void *data = (void *)hp->rl[i]->list[j];
+		for (int j = 0; j < rl->size; j++) {
+			void *data = (void *)rl->list[j];
 			hp->deletefunc(data);
 		}
-		isc_mem_put(hp->mctx, hp->rl[i]->list,
-			    isc__hp_max_retired * sizeof(uintptr_t));
-		isc_mem_put(hp->mctx, hp->rl[i], sizeof(*hp->rl[0]));
+		isc_mem_put(hp->mctx, rl->list,
+			    hp->max_retired * sizeof(uintptr_t));
+		isc_mem_put(hp->mctx, rl, sizeof(*rl));
+	}
+	for (int i = 0; i < isc__hp_max_threads; i++) {
+		isc_mem_put(hp->mctx, hp->hp[i], hp_clpad(hp->max_hps));
 	}
 	isc_mem_put(hp->mctx, hp->hp, isc__hp_max_threads * sizeof(hp->hp[0]));
-	isc_mem_put(hp->mctx, hp->rl, isc__hp_max_threads * sizeof(hp->rl[0]));
+	isc_mem_put(hp->mctx, hp->rl,
+		    (isc__hp_max_threads * CLPAD) * sizeof(hp->rl[0]));
 
 	isc_mem_putanddetach(&hp->mctx, hp, sizeof(*hp));
 }
@@ -173,15 +213,16 @@ isc_hp_protect_release(isc_hp_t *hp, int ihp, atomic_uintptr_t ptr) {
 
 void
 isc_hp_retire(isc_hp_t *hp, uintptr_t ptr) {
-	hp->rl[tid()]->list[hp->rl[tid()]->size++] = ptr;
-	INSIST(hp->rl[tid()]->size < isc__hp_max_retired);
+	retirelist_t *rl = hp->rl[tid() * CLPAD];
+	rl->list[rl->size++] = ptr;
+	INSIST(rl->size < hp->max_retired);
 
-	if (hp->rl[tid()]->size < HP_THRESHOLD_R) {
+	if (rl->size < HP_THRESHOLD_R) {
 		return;
 	}
 
-	for (int iret = 0; iret < hp->rl[tid()]->size; iret++) {
-		uintptr_t obj = hp->rl[tid()]->list[iret];
+	for (int iret = 0; iret < rl->size; iret++) {
+		uintptr_t obj = rl->list[iret];
 		bool can_delete = true;
 		for (int itid = 0; itid < isc__hp_max_threads && can_delete;
 		     itid++) {
@@ -194,11 +235,9 @@ isc_hp_retire(isc_hp_t *hp, uintptr_t ptr) {
 		}
 
 		if (can_delete) {
-			size_t bytes = (hp->rl[tid()]->size - iret) *
-				       sizeof(hp->rl[tid()]->list[0]);
-			memmove(&hp->rl[tid()]->list[iret],
-				&hp->rl[tid()]->list[iret + 1], bytes);
-			hp->rl[tid()]->size--;
+			size_t bytes = (rl->size - iret) * sizeof(rl->list[0]);
+			memmove(&rl->list[iret], &rl->list[iret + 1], bytes);
+			rl->size--;
 			hp->deletefunc((void *)obj);
 		}
 	}

lib/isc/include/isc/mem.h

@@ -117,7 +117,7 @@ LIBISC_EXTERNAL_DATA extern unsigned int isc_mem_defaultflags;
  */
 
 #if !defined(ISC_MEM_USE_INTERNAL_MALLOC) && !__SANITIZE_ADDRESS__
-#define ISC_MEM_USE_INTERNAL_MALLOC 1
+#define ISC_MEM_USE_INTERNAL_MALLOC 0
 #endif /* ifndef ISC_MEM_USE_INTERNAL_MALLOC */
 
 /*

lib/isc/netmgr/netmgr.c

@@ -425,6 +425,7 @@ nm_destroy(isc_nm_t **mgr0) {
 			isc_mempool_put(mgr->evpool, ievent);
 		}
 		isc_condition_destroy(&worker->cond_prio);
+		isc_mutex_destroy(&worker->lock);
 
 		r = uv_loop_close(&worker->loop);
 		INSIST(r == 0);
@@ -1267,8 +1268,9 @@ nmsocket_cleanup(isc_nmsocket_t *sock, bool dofree FLARG) {
 
 	isc_mem_free(sock->mgr->mctx, sock->ah_frees);
 	isc_mem_free(sock->mgr->mctx, sock->ah_handles);
-	isc_mutex_destroy(&sock->lock);
 	isc_condition_destroy(&sock->scond);
+	isc_condition_destroy(&sock->cond);
+	isc_mutex_destroy(&sock->lock);
 #ifdef NETMGR_TRACE
 	LOCK(&sock->mgr->lock);
 	ISC_LIST_UNLINK(sock->mgr->active_sockets, sock, active_link);

lib/isc/openssl_shim.c

@@ -220,3 +220,10 @@ OPENSSL_init_ssl(uint64_t opts, const void *settings) {
 	return (1);
 }
 #endif
+
+#if !HAVE_OPENSSL_CLEANUP
+void
+OPENSSL_cleanup(void) {
+	return;
+}
+#endif

lib/isc/openssl_shim.h

@@ -119,6 +119,11 @@ OPENSSL_init_ssl(uint64_t opts, const void *settings);
 
 #endif
 
+#if !HAVE_OPENSSL_CLEANUP
+void
+OPENSSL_cleanup(void);
+#endif
+
 #if !HAVE_TLS_SERVER_METHOD
 #define TLS_server_method SSLv23_server_method
 #endif

lib/isc/tls.c

@@ -13,6 +13,7 @@
 
 #include <openssl/bn.h>
 #include <openssl/conf.h>
+#include <openssl/crypto.h>
 #include <openssl/err.h>
 #include <openssl/opensslv.h>
 #include <openssl/rand.h>
@@ -123,8 +124,9 @@ tls_shutdown(void) {
 	REQUIRE(atomic_load(&init_done));
 	REQUIRE(!atomic_load(&shut_done));
 
-#if OPENSSL_VERSION_NUMBER < 0x10100000L
-
+#if OPENSSL_VERSION_NUMBER >= 0x10100000L
+	OPENSSL_cleanup();
+#else
 	CONF_modules_unload(1);
 	OBJ_cleanup();
 	EVP_cleanup();