Mercurial > illumos > illumos-gate

changeset 4914:8e96a2e351b2

Find changesets by keywords (author, files, the commit message), revision number or hash, or revset expression.
6593908 fixes for 6518780 cause more deadlocks
author raf
date Mon, 20 Aug 2007 17:36:02 -0700
parents 8d95fa5f1def
children b6d0b03690de
files usr/src/lib/libc/inc/libc.h usr/src/lib/libc/port/gen/getnetgrent.c usr/src/lib/libc/port/gen/ttyname.c usr/src/lib/libc/port/i18n/gettext.c usr/src/lib/libc/port/i18n/wdresolve.c usr/src/lib/libc/port/threads/scalls.c
diffstat 6 files changed, 102 insertions(+), 76 deletions(-) [+]
line wrap: on
line diff
--- a/usr/src/lib/libc/inc/libc.h	Mon Aug 20 17:13:57 2007 -0700
+++ b/usr/src/lib/libc/inc/libc.h	Mon Aug 20 17:36:02 2007 -0700
@@ -68,8 +68,8 @@
 extern thread_t thr_self(void);
 extern int mutex_lock(mutex_t *mp);
 extern int mutex_unlock(mutex_t *mp);
-extern void fork_lock_enter(void);
-extern void fork_lock_exit(void);
+extern void atfork_lock_enter(void);
+extern void atfork_lock_exit(void);
 extern void *lmalloc(size_t);
 extern void lfree(void *, size_t);
 extern void *libc_malloc(size_t);
--- a/usr/src/lib/libc/port/gen/getnetgrent.c	Mon Aug 20 17:13:57 2007 -0700
+++ b/usr/src/lib/libc/port/gen/getnetgrent.c	Mon Aug 20 17:36:02 2007 -0700
@@ -137,7 +137,7 @@
  * serialize them ourselves (anything to prevent a coredump)...
  * We can't use lmutex_lock() here because we don't know what the backends
  * that we call may call in turn.  They might call malloc()/free().
- * So we use the brute-force fork_lock_enter() instead.
+ * So we use the brute-force atfork_lock_enter() instead.
  */
 static nss_backend_t	*getnetgrent_backend;
 
@@ -151,7 +151,7 @@
 		netgroup = "";
 	}
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	be = getnetgrent_backend;
 	if (be != NULL && NSS_INVOKE_DBOP(be, NSS_DBOP_SETENT,
 	    (void *)netgroup) != NSS_SUCCESS) {
@@ -168,7 +168,7 @@
 		be = args.iterator;
 	}
 	getnetgrent_backend = be;
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (0);
 }
 
@@ -186,12 +186,12 @@
 	args.buflen	= buflen;
 	args.status	= NSS_NETGR_NO;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	if (getnetgrent_backend != 0) {
 		(void) NSS_INVOKE_DBOP(getnetgrent_backend,
 			NSS_DBOP_GETENT, &args);
 	}
-	fork_lock_exit();
+	atfork_lock_exit();
 
 	if (args.status == NSS_NETGR_FOUND) {
 		*machinep = args.retp[NSS_NETGR_MACHINE];
@@ -219,13 +219,13 @@
 int
 endnetgrent()
 {
-	fork_lock_enter();
+	atfork_lock_enter();
 	if (getnetgrent_backend != 0) {
 		(void) NSS_INVOKE_DBOP(getnetgrent_backend,
 		    NSS_DBOP_DESTRUCTOR, 0);
 		getnetgrent_backend = 0;
 	}
-	fork_lock_exit();
+	atfork_lock_exit();
 	nss_delete(&db_root);	/* === ? */
 	NSS_XbyY_FREE(&buf);
 	return (0);
--- a/usr/src/lib/libc/port/gen/ttyname.c	Mon Aug 20 17:13:57 2007 -0700
+++ b/usr/src/lib/libc/port/gen/ttyname.c	Mon Aug 20 17:36:02 2007 -0700
@@ -213,9 +213,9 @@
 
 	/*
 	 * We can't use lmutex_lock() here because we call malloc()/free()
-	 * and _libc_gettext().  Use the brute-force fork_lock_enter().
+	 * and _libc_gettext().  Use the brute-force atfork_lock_enter().
 	 */
-	fork_lock_enter();
+	atfork_lock_enter();
 
 	/*
 	 * match special cases
@@ -320,7 +320,7 @@
 	else
 		retval = NULL;
 out:	retval = (retval ? strcpy(buffer, retval) : NULL);
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (retval);
 }
 
--- a/usr/src/lib/libc/port/i18n/gettext.c	Mon Aug 20 17:13:57 2007 -0700
+++ b/usr/src/lib/libc/port/i18n/gettext.c	Mon Aug 20 17:36:02 2007 -0700
@@ -59,7 +59,7 @@
 		if (global_gt) \
 			global_gt->cur_domain = (char *)default_domain; \
 		else { \
-			fork_lock_exit(); \
+			atfork_lock_exit(); \
 			return ((def)); \
 		} \
 	}
@@ -73,10 +73,10 @@
 {
 	char	*res;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT(NULL);
 	res = _real_bindtextdomain_u(domain, binding, TP_BINDING);
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (res);
 }
 
@@ -85,10 +85,10 @@
 {
 	char	*res;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT(NULL);
 	res = _real_bindtextdomain_u(domain, codeset, TP_CODESET);
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (res);
 }
 
@@ -102,14 +102,14 @@
 	char	*res;
 	char	tmp_domain[TEXTDOMAINMAX + 1];
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT(NULL);
 	res = _textdomain_u(domain, tmp_domain);
 	if (res == NULL) {
-		fork_lock_exit();
+		atfork_lock_exit();
 		return (NULL);
 	}
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (CURRENT_DOMAIN(global_gt));
 }
 
@@ -123,10 +123,10 @@
 	char	*res;
 	int	errno_save = errno;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT((char *)msg_id);
 	res = _real_gettext_u(NULL, msg_id, NULL, 0, LC_MESSAGES, 0);
-	fork_lock_exit();
+	atfork_lock_exit();
 	errno = errno_save;
 	return (res);
 }
@@ -141,10 +141,10 @@
 	char	*res;
 	int	errno_save = errno;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT((char *)msg_id);
 	res = _real_gettext_u(domain, msg_id, NULL, 0, LC_MESSAGES, 0);
-	fork_lock_exit();
+	atfork_lock_exit();
 	errno = errno_save;
 	return (res);
 }
@@ -155,10 +155,10 @@
 	char	*res;
 	int	errno_save = errno;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT((char *)msg_id);
 	res = _real_gettext_u(domain, msg_id, NULL, 0, category, 0);
-	fork_lock_exit();
+	atfork_lock_exit();
 	errno = errno_save;
 	return (res);
 }
@@ -169,10 +169,10 @@
 	char	*res;
 	int	errno_save = errno;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT((char *)msgid1);
 	res = _real_gettext_u(NULL, msgid1, msgid2, n, LC_MESSAGES, 1);
-	fork_lock_exit();
+	atfork_lock_exit();
 	errno = errno_save;
 	return (res);
 }
@@ -184,10 +184,10 @@
 	char	*res;
 	int	errno_save = errno;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT((char *)msgid1);
 	res = _real_gettext_u(domain, msgid1, msgid2, n, LC_MESSAGES, 1);
-	fork_lock_exit();
+	atfork_lock_exit();
 	errno = errno_save;
 	return (res);
 }
@@ -199,10 +199,10 @@
 	char	*res;
 	int	errno_save = errno;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	INIT_GT((char *)msgid1);
 	res = _real_gettext_u(domain, msgid1, msgid2, n, category, 1);
-	fork_lock_exit();
+	atfork_lock_exit();
 	errno = errno_save;
 	return (res);
 }
--- a/usr/src/lib/libc/port/i18n/wdresolve.c	Mon Aug 20 17:13:57 2007 -0700
+++ b/usr/src/lib/libc/port/i18n/wdresolve.c	Mon Aug 20 17:36:02 2007 -0700
@@ -120,9 +120,9 @@
 {
 	int res;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	res = _wdinitialize();
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (res);
 }
 
@@ -135,11 +135,11 @@
 {
 	int i;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	if (!initialized)
 		(void) _wdinitialize();
 	i = (*wdchknd)(wc);
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (i);
 }
 static int
@@ -170,15 +170,15 @@
 {
 	int i;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	if (!initialized)
 		(void) _wdinitialize();
 	if (!iswprint(wc1) || !iswprint(wc2)) {
-		fork_lock_exit();
+		atfork_lock_exit();
 		return (-1);
 	}
 	i = (*wdbdg)(wc1, wc2, type);
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (i);
 }
 /*ARGSUSED*/
@@ -203,15 +203,15 @@
 {
 	wchar_t *i;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	if (!initialized)
 		(void) _wdinitialize();
 	if (!iswprint(wc1) || !iswprint(wc2)) {
-		fork_lock_exit();
+		atfork_lock_exit();
 		return ((wchar_t *)L"");
 	}
 	i = (*wddlm)(wc1, wc2, type);
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (i);
 }
 /*ARGSUSED*/
@@ -230,7 +230,7 @@
 {
 	wchar_t fillerchar;
 
-	fork_lock_enter();
+	atfork_lock_enter();
 	if (!initialized)
 		(void) _wdinitialize();
 	if (mcfllr) {
@@ -238,11 +238,11 @@
 		if (!fillerchar)
 			fillerchar = (wchar_t)'~';
 		if (iswprint(fillerchar)) {
-			fork_lock_exit();
+			atfork_lock_exit();
 			return (fillerchar);
 		}
 	}
-	fork_lock_exit();
+	atfork_lock_exit();
 	return ((wchar_t)'~');
 }
 
@@ -254,14 +254,14 @@
 int
 mcwrap(void)
 {
-	fork_lock_enter();
+	atfork_lock_enter();
 	if (!initialized)
 		(void) _wdinitialize();
 	if (mcwrp)
 		if ((*mcwrp)() == 0) {
-			fork_lock_exit();
+			atfork_lock_exit();
 			return (0);
 		}
-	fork_lock_exit();
+	atfork_lock_exit();
 	return (1);
 }
--- a/usr/src/lib/libc/port/threads/scalls.c	Mon Aug 20 17:13:57 2007 -0700
+++ b/usr/src/lib/libc/port/threads/scalls.c	Mon Aug 20 17:36:02 2007 -0700
@@ -35,28 +35,56 @@
 #include <sys/uio.h>
 
 /*
- * fork_lock_enter() does triple-duty.  Not only does it (and atfork_lock)
- * serialize calls to fork() and forkall(), but it also serializes calls to
- * thr_suspend() and thr_continue() (fork() and forkall() also suspend other
- * threads), and furthermore it serializes I18N calls to functions in other
- * dlopen()ed L10N objects that might be calling malloc()/free().
+ * fork_lock does double-duty.  Not only does it (and atfork_lock)
+ * serialize calls to fork() and forkall(), but it also serializes calls
+ * to thr_suspend() and thr_continue() (because fork() and forkall() also
+ * suspend and continue other threads and they want no competition).
+ *
+ * atfork_lock also does double-duty.  Not only does it protect the
+ * pthread_atfork() data structures, but it also serializes I18N calls
+ * to functions in dlopen()ed L10N objects.  These functions can do
+ * anything, including call malloc() and free().  Such calls are not
+ * fork-safe when protected by an ordinary mutex because, with an
+ * interposed malloc library present, there would be a lock ordering
+ * violation due to the pthread_atfork() prefork function in the
+ * interposition library acquiring its malloc lock(s) before the
+ * ordinary mutex in libc being acquired by libc's prefork functions.
+ *
+ * Within libc, calls to malloc() and free() are fork-safe only if the
+ * calls are made while holding no other libc locks.  This covers almost
+ * all of libc's malloc() and free() calls.  For those libc code paths,
+ * such as the above-mentioned I18N calls, that require serialization and
+ * that may call malloc() or free(), libc uses atfork_lock_enter() to perform
+ * the serialization.  This works because atfork_lock is acquired by fork()
+ * before any of the pthread_atfork() prefork functions are called.
  */
+
 void
 fork_lock_enter(void)
 {
-	ulwp_t *self = curthread;
-
-	ASSERT(self->ul_critical == 0);
-	(void) _private_mutex_lock(&self->ul_uberdata->fork_lock);
+	ASSERT(curthread->ul_critical == 0);
+	(void) _private_mutex_lock(&curthread->ul_uberdata->fork_lock);
 }
 
 void
 fork_lock_exit(void)
 {
-	ulwp_t *self = curthread;
+	ASSERT(curthread->ul_critical == 0);
+	(void) _private_mutex_unlock(&curthread->ul_uberdata->fork_lock);
+}
 
-	ASSERT(self->ul_critical == 0);
-	(void) _private_mutex_unlock(&self->ul_uberdata->fork_lock);
+void
+atfork_lock_enter(void)
+{
+	ASSERT(curthread->ul_critical == 0);
+	(void) _private_mutex_lock(&curthread->ul_uberdata->atfork_lock);
+}
+
+void
+atfork_lock_exit(void)
+{
+	ASSERT(curthread->ul_critical == 0);
+	(void) _private_mutex_unlock(&curthread->ul_uberdata->atfork_lock);
 }
 
 #pragma weak forkx = _private_forkx
@@ -97,7 +125,6 @@
 		return (-1);
 	}
 	self->ul_fork = 1;
-	(void) _private_mutex_lock(&udp->atfork_lock);
 
 	/*
 	 * The functions registered by pthread_atfork() are defined by
@@ -105,20 +132,18 @@
 	 * internal lmutex_lock()-acquired locks while invoking them.
 	 * We hold only udp->atfork_lock to protect the atfork linkages.
 	 * If one of these pthread_atfork() functions attempts to fork
-	 * or to call pthread_atfork(), it will detect the error and
-	 * fail with EDEADLK.  Otherwise, the pthread_atfork() functions
-	 * are free to do anything they please (except they will not
-	 * receive any signals).
+	 * or to call pthread_atfork(), libc will detect the error and
+	 * fail the call with EDEADLK.  Otherwise, the pthread_atfork()
+	 * functions are free to do anything they please (except they
+	 * will not receive any signals).
 	 */
+	(void) _private_mutex_lock(&udp->atfork_lock);
 	_prefork_handler();
 
 	/*
 	 * Block every other thread attempting thr_suspend() or thr_continue().
-	 * This also blocks every other thread attempting calls to I18N
-	 * functions in dlopen()ed L10N objects, but this is benign;
-	 * the other threads will soon be suspended anyway.
 	 */
-	fork_lock_enter();
+	(void) _private_mutex_lock(&udp->fork_lock);
 
 	/*
 	 * Block all signals.
@@ -131,8 +156,8 @@
 	/*
 	 * This suspends all threads but this one, leaving them
 	 * suspended outside of any critical regions in the library.
-	 * Thus, we are assured that no library locks are held
-	 * while we invoke fork() from the current thread.
+	 * Thus, we are assured that no lmutex_lock()-acquired library
+	 * locks are held while we invoke fork() from the current thread.
 	 */
 	suspend_fork();
 
@@ -154,13 +179,13 @@
 		unregister_locks();
 		postfork1_child();
 		restore_signals(self);
-		fork_lock_exit();
+		(void) _private_mutex_unlock(&udp->fork_lock);
 		_postfork_child_handler();
 	} else {
 		/* restart all threads that were suspended for fork() */
 		continue_fork(0);
 		restore_signals(self);
-		fork_lock_exit();
+		(void) _private_mutex_unlock(&udp->fork_lock);
 		_postfork_parent_handler();
 	}
 
@@ -218,8 +243,8 @@
 		return (-1);
 	}
 	self->ul_fork = 1;
-
-	fork_lock_enter();
+	(void) _private_mutex_lock(&udp->atfork_lock);
+	(void) _private_mutex_lock(&udp->fork_lock);
 	block_all_signals(self);
 	suspend_fork();
 
@@ -237,7 +262,8 @@
 		continue_fork(0);
 	}
 	restore_signals(self);
-	fork_lock_exit();
+	(void) _private_mutex_unlock(&udp->fork_lock);
+	(void) _private_mutex_unlock(&udp->atfork_lock);
 	self->ul_fork = 0;
 	sigon(self);