Block library unloading to avoid finalizer races

Library finalizers can run due to the library being unloaded or the process exiting. If the library is being unloaded, global memory resources must be released to avoid memory leaks. But if the process is exiting, releasing memory resources can lead to race conditions if another thread invokes functions from the library during or after finalizer execution. Most commonly this manifests as an assertion error about trying to lock a destroyed mutex. We can block unloading of our library on ELF platforms by passing "-z nodelete" to the linker. Add a shell variable "lib_unload_prevented" to the shlib.conf outputs, set it on platforms where we can block unloading, and suppress finalizers when it is set. On Windows we can detect if the process is exiting by checking for a non-null lpvReserved argument in DllMain(). Do not execute finalizers when the process is executing. ticket: 9139 (new)
greghudson · Sep 10, 2024 · 6112018 · 6112018
1 parent b9b654e
commit 6112018
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 16 deletions.
diff --git a/src/aclocal.m4 b/src/aclocal.m4
@@ -139,6 +139,9 @@ fi
 if test "$use_linker_fini_option" = yes; then
   AC_DEFINE(USE_LINKER_FINI_OPTION,1,[Define if link-time options for library finalization will be used])
 fi
+if test "$lib_unload_prevented" = yes; then
+  AC_DEFINE(LIB_UNLOAD_PREVENTED,1,[Define if library unloading is prevented])
+fi
 ])
 
 dnl find dlopen

diff --git a/src/config/shlib.conf b/src/config/shlib.conf
@@ -32,9 +32,15 @@ DYNOBJEXT='$(SHLIBEXT)'
 MAKE_DYNOBJ_COMMAND='$(MAKE_SHLIB_COMMAND)'
 DYNOBJ_EXPDEPS='$(SHLIB_EXPDEPS)'
 DYNOBJ_EXPFLAGS='$(SHLIB_EXPFLAGS)'
-#
+# On some platforms we will instruct the linker to run named functions
+# (specified by LIBINITFUNC and LIBFINIFUNC in each library's Makefile.in) as
+# initializers or finalizers.
 use_linker_init_option=no
 use_linker_fini_option=no
+# Where possible we will prevent unloading of the libraries we build, in which
+# case we can skip running finalizers.  Do not set use_linker_fini_option if
+# setting lib_unload_prevented.
+lib_unload_prevented=no
 
 STOBJEXT=.o
 SHOBJEXT=.so
@@ -254,10 +260,10 @@ mips-sgi-irix*)
 mips-sni-sysv4)
 	if test "$ac_cv_c_compiler_gnu" = yes; then
 		PICFLAGS=-fpic
-		LDCOMBINE='$(CC) -G -Wl,-h -Wl,$(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
+		LDCOMBINE='$(CC) -G -Wl,-h -Wl,$(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) -Wl,-z,nodelete'
 	else
 		PICFLAGS=-Kpic
-		LDCOMBINE='$(CC) -G -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
+		LDCOMBINE='$(CC) -G -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) -Wl,-z,nodelete'
 	fi
 	SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
 	SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
@@ -273,14 +279,15 @@ mips-sni-sysv4)
 	RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
 	PROFFLAGS=-pg
+	lib_unload_prevented=yes
 	;;
 
 mips-*-netbsd*)
 	PICFLAGS=-fPIC
 	SHLIBVEXT='.so.$(LIBMAJOR).$(LIBMINOR)'
 	SHLIBSEXT='.so.$(LIBMAJOR)'
 	SHLIBEXT=.so
-	LDCOMBINE='ld -shared -soname $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
+	LDCOMBINE='ld -shared -soname $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) -z nodelete'
 	SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
 	SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
 	RPATH_FLAG='-Wl,-rpath -Wl,'
@@ -292,13 +299,14 @@ mips-*-netbsd*)
 	RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
 	PROFFLAGS=-pg
+	lib_unload_prevented=yes
 	;;
 
 *-*-netbsd*)
 	PICFLAGS=-fPIC
 	SHLIBVEXT='.so.$(LIBMAJOR).$(LIBMINOR)'
 	SHLIBEXT=.so
-	LDCOMBINE='$(CC) -shared'
+	LDCOMBINE='$(CC) -shared -Wl,-z,nodelete'
 	SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
 	SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
 	RPATH_FLAG=-R
@@ -310,6 +318,7 @@ mips-*-netbsd*)
 	RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
 	PROFFLAGS=-pg
+	lib_unload_prevented=yes
 	;;
 
 *-*-freebsd*)
@@ -327,21 +336,22 @@ mips-*-netbsd*)
 	CC_LINK_SHARED='$(CC) $(PROG_LIBPATH) $(PROG_RPATH_FLAGS) $(CFLAGS) $(LDFLAGS)'
 	CXX_LINK_SHARED='$(CXX) $(PROG_LIBPATH) $(PROG_RPATH_FLAGS) $(CXXFLAGS) $(LDFLAGS)'
 	SHLIBEXT=.so
-	LDCOMBINE='ld -Bshareable'
+	LDCOMBINE='ld -Bshareable -z nodelete'
 	SHLIB_RPATH_FLAGS='--enable-new-dtags -rpath $(SHLIB_RDIRS)'
 	SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
 	CC_LINK_STATIC='$(CC) $(PROG_LIBPATH) $(CFLAGS) $(LDFLAGS)'
 	CXX_LINK_STATIC='$(CXX) $(PROG_LIBPATH) $(CXXFLAGS) $(LDFLAGS)'
 	RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
 	PROFFLAGS=-pg
+	lib_unload_prevented=yes
 	;;
 
 *-*-openbsd*)
 	PICFLAGS=-fpic
 	SHLIBVEXT='.so.$(LIBMAJOR).$(LIBMINOR)'
 	SHLIBEXT=.so
-	LDCOMBINE='ld -Bshareable'
+	LDCOMBINE='ld -Bshareable -z nodelete'
 	SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
 	SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
 	RPATH_FLAG=-R
@@ -353,6 +363,7 @@ mips-*-netbsd*)
 	RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
 	PROFFLAGS=-pg
+	lib_unload_prevented=yes
 	;;
 
 *-*-darwin* | *-*-rhapsody*)
@@ -383,20 +394,19 @@ mips-*-netbsd*)
 *-*-solaris*)
 	if test "$ac_cv_c_compiler_gnu" = yes; then
 		PICFLAGS=-fPIC
-		LDCOMBINE='$(CC) $(CFLAGS) -shared -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
+		LDCOMBINE='$(CC) $(CFLAGS) -shared -Wl,-z,nodelete -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT)'
 	else
 		PICFLAGS=-KPIC
 		# Solaris cc doesn't default to stuffing the SONAME field...
-		LDCOMBINE='$(CC) $(CFLAGS) -dy -G -z text -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $$initfini'
+		LDCOMBINE='$(CC) $(CFLAGS) -dy -G -z text -z nodelete -h $(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $$initfini'
 		#
 		case $krb5_cv_host in
 		*-*-solaris2.[1-7] | *-*-solaris2.[1-7].*)
 		    # Did Solaris 7 and earlier have a linker option for this?
 		    ;;
 		*)
-		    INIT_FINI_PREP='initfini=; for f in . $(LIBINITFUNC); do if test $$f = .; then :; else initfini="$$initfini -Wl,-z,initarray=$${f}__auxinit"; fi; done; for f in . $(LIBFINIFUNC); do if test $$f = .; then :; else initfini="$$initfini -Wl,-z,finiarray=$$f"; fi; done'
+		    INIT_FINI_PREP='initfini=; for f in . $(LIBINITFUNC); do if test $$f = .; then :; else initfini="$$initfini -Wl,-z,initarray=$${f}__auxinit"; fi; done;'
 		    use_linker_init_option=yes
-		    use_linker_fini_option=yes
 		    ;;
 		esac
 	fi
@@ -414,6 +424,7 @@ mips-*-netbsd*)
 	CXX_LINK_STATIC='$(PURE) $(CXX) $(PROG_LIBPATH) $(CXXFLAGS) $(LDFLAGS)'
 	RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
+	lib_unload_prevented=yes
 	;;
 
 *-*-linux* | *-*-gnu* | *-*-k*bsd*-gnu)
@@ -424,7 +435,7 @@ mips-*-netbsd*)
 	# Linux ld doesn't default to stuffing the SONAME field...
 	# Use objdump -x to examine the fields of the library
 	# UNDEF_CHECK is suppressed by --enable-asan
-	LDCOMBINE='$(CC) -shared -fPIC -Wl,-h,$(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $(UNDEF_CHECK)'
+	LDCOMBINE='$(CC) -shared -fPIC -Wl,-z,nodelete -Wl,-h,$(LIBPREFIX)$(LIBBASE)$(SHLIBSEXT) $(UNDEF_CHECK)'
 	UNDEF_CHECK='-Wl,--no-undefined'
 	# $(EXPORT_CHECK) runs export-check.pl when in maintainer mode.
 	LDCOMBINE_TAIL='-Wl,--version-script binutils.versions $(EXPORT_CHECK)'
@@ -442,6 +453,7 @@ mips-*-netbsd*)
 	CXX_LINK_STATIC='$(CXX) $(PROG_LIBPATH) $(CXXFLAGS) $(LDFLAGS)'
 	RUN_ENV='LD_LIBRARY_PATH=`echo $(PROG_LIBPATH) | sed -e "s/-L//g" -e "s/ /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
+	lib_unload_prevented=yes
 
 	## old version:
 	# Linux libc does weird stuff at shlib link time, must be
@@ -459,7 +471,7 @@ mips-*-netbsd*)
 	PICFLAGS=-fpic
 	SHLIBVEXT='.so.$(LIBMAJOR)'
 	SHLIBEXT=.so
-	LDCOMBINE='ld -Bshareable'
+	LDCOMBINE='ld -Bshareable -z nodelete'
 	SHLIB_RPATH_FLAGS='-R$(SHLIB_RDIRS)'
 	SHLIB_EXPFLAGS='$(SHLIB_RPATH_FLAGS) $(SHLIB_DIRS) $(SHLIB_EXPLIBS)'
 	PROG_RPATH_FLAGS='-Wl,-rpath,$(PROG_RPATH)'
@@ -471,6 +483,7 @@ mips-*-netbsd*)
 /:/g"`'
 	RUN_VARS='LD_LIBRARY_PATH'
 	PROFFLAGS=-pg
+	lib_unload_prevented=yes
 	;;
 
 *-*-aix5*)

diff --git a/src/include/k5-platform.h b/src/include/k5-platform.h
@@ -153,6 +153,9 @@
    doing the pthread test at run time on systems where that works, so
    we use the k5_once_t stuff instead.)
 
+   UNIX, with library unloading prevented or when building static
+   libraries: we don't need to run finalizers.
+
    UNIX, with compiler support: MAKE_FINI_FUNCTION declares the
    function as a destructor, and the run time linker support or
    whatever will cause it to be invoked when the library is unloaded,
@@ -398,7 +401,7 @@ typedef struct { int error; unsigned char did_run; } k5_init_t;
 
 # endif
 
-#elif !defined(SHARED)
+#elif !defined(SHARED) || defined(LIB_UNLOAD_PREVENTED)
 
 /*
  * In this case, we just don't care about finalization.  The code will still

diff --git a/src/lib/win_glue.c b/src/lib/win_glue.c
@@ -401,7 +401,7 @@ control(int mode)
 
 #ifdef _WIN32
 
-BOOL WINAPI DllMain (HANDLE hModule, DWORD fdwReason, LPVOID lpReserved)
+BOOL WINAPI DllMain (HANDLE hModule, DWORD fdwReason, LPVOID lpvReserved)
 {
     switch (fdwReason)
     {
@@ -420,7 +420,9 @@ BOOL WINAPI DllMain (HANDLE hModule, DWORD fdwReason, LPVOID lpReserved)
 	    break;
 
         case DLL_PROCESS_DETACH:
-	    if (control(DLL_SHUTDOWN))
+	    /* If lpvReserved is non-null, the process is exiting and we do not
+	     * need to clean up library memory. */
+	    if (lpvReserved == NULL && control(DLL_SHUTDOWN))
 		return FALSE;
 	    break;