From owner-freebsd-threads@FreeBSD.ORG Tue Dec 11 07:40:03 2007 Return-Path: Delivered-To: freebsd-threads@hub.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 3D48C16A418 for ; Tue, 11 Dec 2007 07:40:03 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 1F17F13C46A for ; Tue, 11 Dec 2007 07:40:03 +0000 (UTC) (envelope-from gnats@FreeBSD.org) Received: from freefall.freebsd.org (gnats@localhost [127.0.0.1]) by freefall.freebsd.org (8.14.2/8.14.2) with ESMTP id lBB7e2mB097638 for ; Tue, 11 Dec 2007 07:40:02 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.2/8.14.1/Submit) id lBB7e2o5097637; Tue, 11 Dec 2007 07:40:02 GMT (envelope-from gnats) Resent-Date: Tue, 11 Dec 2007 07:40:02 GMT Resent-Message-Id: <200712110740.lBB7e2o5097637@freefall.freebsd.org> Resent-From: FreeBSD-gnats-submit@FreeBSD.org (GNATS Filer) Resent-To: freebsd-threads@FreeBSD.org Resent-Reply-To: FreeBSD-gnats-submit@FreeBSD.org, Changming Sun Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0C00216A418 for ; Tue, 11 Dec 2007 07:38:55 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (www.freebsd.org [IPv6:2001:4f8:fff6::21]) by mx1.freebsd.org (Postfix) with ESMTP id F159713C46A for ; Tue, 11 Dec 2007 07:38:54 +0000 (UTC) (envelope-from nobody@FreeBSD.org) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.2/8.14.2) with ESMTP id lBB7cpj3007040 for ; Tue, 11 Dec 2007 07:38:51 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.2/8.14.1/Submit) id lBB7cpB9007039; Tue, 11 Dec 2007 07:38:51 GMT (envelope-from nobody) Message-Id: <200712110738.lBB7cpB9007039@www.freebsd.org> Date: Tue, 11 Dec 2007 07:38:51 GMT From: Changming Sun To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: threads/118544: the "clnt_create" function in "libc/rpc" is not multi-thread safe X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 11 Dec 2007 07:40:03 -0000 >Number: 118544 >Category: threads >Synopsis: the "clnt_create" function in "libc/rpc" is not multi-thread safe >Confidential: no >Severity: non-critical >Priority: medium >Responsible: freebsd-threads >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Dec 11 07:40:02 UTC 2007 >Closed-Date: >Last-Modified: >Originator: Changming Sun >Release: Freebsd 6.2 >Organization: NASDAQ:SINA >Environment: FreeBSD localhost 6.2-STORM-r4 FreeBSD 6.2-STORM-r4 #0: Thu Feb 1 10:06:48 UTC 2007 root@mammoth.sina.com:/usr/obj/usr/src/sys/MPSINAMAIL i386 i386 Intel(R) Xeon(TM) CPU 3.06GHz FreeBSD >Description: I've write a multi-threaded program which is running under Freebsd,it invokes the "clnt_create" function in many threads (not only the main thread),and it got core dump sometimes. Here is the backtrace: #0 0×84c7f82a in fclose (fp=0×0) at /usr/src/lib/libc/stdio/fclose.c:56 #1 0×84c4b0a2 in endnetconfig (handlep=0×86e0420) at /usr/src/lib/libc/rpc/getnetconfig.c:394 #2 0×84c40cc5 in __rpc_endconf (vhandle=0×86e0410) at /usr/src/lib/libc/rpc/rpc_generic.c:441 #3 0×84c327eb in clnt_create_timed (hostname=0×80977d8 “127.0.0.1″, prog=931729681, vers=1, netclass=0×80977d4 “tcp”, tp=0×0) at /usr/src/lib/libc/rpc/clnt_generic.c:271 #4 0×84c3264d in clnt_create (hostname=0×80977d8 “127.0.0.1″, prog=931729681, vers=1, nettype=0×80977d4 “tcp”) at /usr/src/lib/libc/rpc/clnt_generic.c:186 [...] Then I found that the "clnt_create" function is not multi-thread safe.There are some global variables in "/usr/src/lib/libc/rpc/getnetconfig.c",and we read/modify it without locking. >How-To-Repeat: It is hard to repeat. write such a multi-threaded program,and try. >Fix: This is a patch to Freebsd 6.2 release. --- src/lib/libc/rpc/getnetconfig.c.orig Tue Dec 11 13:45:32 2007 +++ src/lib/libc/rpc/getnetconfig.c Tue Dec 11 14:11:41 2007 @@ -131,7 +131,10 @@ static struct netconfig *dup_ncp(struct static FILE *nc_file; /* for netconfig db */ +static pthread_mutex_t nc_file_lock = PTHREAD_MUTEX_INITIALIZER; static struct netconfig_info ni = { 0, 0, NULL, NULL}; +/* should not acquire it after acquired a nc_file_lock */ static +pthread_mutex_t ni_lock = PTHREAD_MUTEX_INITIALIZER; #define MAXNETCONFIGLINE 1000 @@ -205,14 +208,23 @@ setnetconfig() * For multiple calls, i.e. nc_file is not NULL, we just return the * handle without reopening the netconfig db. */ + mutex_lock(&ni_lock); ni.ref++; + mutex_unlock(&ni_lock); + + mutex_lock(&nc_file_lock); if ((nc_file != NULL) || (nc_file = fopen(NETCONFIG, "r")) != NULL) { nc_vars->valid = NC_VALID; nc_vars->flag = 0; nc_vars->nc_configs = ni.head; + mutex_unlock(&nc_file_lock); return ((void *)nc_vars); } + mutex_unlock(&nc_file_lock); + mutex_lock(&ni_lock); ni.ref--; + mutex_unlock(&ni_lock); + nc_error = NC_NONETCONFIG; free(nc_vars); return (NULL); @@ -235,15 +247,17 @@ void *handlep; char *stringp; /* tmp string pointer */ struct netconfig_list *list; struct netconfig *np; - + struct netconfig *result; /* * Verify that handle is valid */ + mutex_lock(&nc_file_lock); if (ncp == NULL || nc_file == NULL) { nc_error = NC_NOTINIT; + mutex_unlock(&nc_file_lock); return (NULL); } - + mutex_unlock(&nc_file_lock); switch (ncp->valid) { case NC_VALID: /* @@ -256,7 +270,9 @@ void *handlep; */ if (ncp->flag == 0) { /* first time */ ncp->flag = 1; + mutex_lock(&ni_lock); ncp->nc_configs = ni.head; + mutex_unlock(&ni_lock); if (ncp->nc_configs != NULL) /* entry already exist */ return(ncp->nc_configs->ncp); } @@ -269,7 +285,12 @@ void *handlep; * If we cannot find the entry in the list and is end of file, * we give up. */ - if (ni.eof == 1) return(NULL); + mutex_lock(&ni_lock); + if (ni.eof == 1) { + mutex_unlock(&ni_lock); + return(NULL); + } + mutex_unlock(&ni_lock); break; default: nc_error = NC_NOTINIT; @@ -290,14 +311,18 @@ void *handlep; /* * Read a line from netconfig file. */ + mutex_lock(&nc_file_lock); do { if (fgets(stringp, MAXNETCONFIGLINE, nc_file) == NULL) { free(stringp); + mutex_lock(&ni_lock); ni.eof = 1; + mutex_unlock(&ni_lock); + mutex_unlock(&nc_file_lock); return (NULL); } } while (*stringp == '#'); - + mutex_unlock(&nc_file_lock); list = (struct netconfig_list *) malloc(sizeof (struct netconfig_list)); if (list == NULL) { free(stringp); @@ -326,6 +351,7 @@ void *handlep; * Reposition the current pointer of the handle to the last entry * in the list. */ + mutex_lock(&ni_lock); if (ni.head == NULL) { /* first entry */ ni.head = ni.tail = list; } @@ -334,7 +360,9 @@ void *handlep; ni.tail = ni.tail->next; } ncp->nc_configs = ni.tail; - return(ni.tail->ncp); + result = ni.tail->ncp; + mutex_unlock(&ni_lock); + return(result); } } @@ -368,8 +396,10 @@ void *handlep; nc_handlep->valid = NC_INVALID; nc_handlep->flag = 0; nc_handlep->nc_configs = NULL; + mutex_lock(&ni_lock); if (--ni.ref > 0) { free(nc_handlep); + mutex_unlock(&ni_lock); return(0); } @@ -381,6 +411,7 @@ void *handlep; ni.eof = ni.ref = 0; ni.head = NULL; ni.tail = NULL; + mutex_unlock(&ni_lock); while (q) { p = q->next; if (q->ncp->nc_lookups != NULL) free(q->ncp->nc_lookups); @@ -390,9 +421,10 @@ void *handlep; q = p; } free(nc_handlep); - + mutex_lock(&nc_file_lock); fclose(nc_file); nc_file = NULL; + mutex_unlock(&nc_file_lock); return (0); } @@ -440,16 +472,20 @@ getnetconfigent(netid) * If all the netconfig db has been read and placed into the list and * there is no match for the netid, return NULL. */ + mutex_lock(&ni_lock); if (ni.head != NULL) { for (list = ni.head; list; list = list->next) { if (strcmp(list->ncp->nc_netid, netid) == 0) { + mutex_unlock(&ni_lock); return(dup_ncp(list->ncp)); } } - if (ni.eof == 1) /* that's all the entries */ + if (ni.eof == 1) {/* that's all the entries */ + mutex_unlock(&ni_lock); return(NULL); + } } - + mutex_unlock(&ni_lock); if ((file = fopen(NETCONFIG, "r")) == NULL) { nc_error = NC_NONETCONFIG; >Release-Note: >Audit-Trail: >Unformatted: