From owner-freebsd-threads@FreeBSD.ORG Mon Sep 20 11:07:06 2010 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 743AC1065747 for ; Mon, 20 Sep 2010 11:07:06 +0000 (UTC) (envelope-from owner-bugmaster@FreeBSD.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 577058FC14 for ; Mon, 20 Sep 2010 11:07:06 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8KB76fY015103 for ; Mon, 20 Sep 2010 11:07:06 GMT (envelope-from owner-bugmaster@FreeBSD.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8KB75iH015101 for freebsd-threads@FreeBSD.org; Mon, 20 Sep 2010 11:07:05 GMT (envelope-from owner-bugmaster@FreeBSD.org) Date: Mon, 20 Sep 2010 11:07:05 GMT Message-Id: <201009201107.o8KB75iH015101@freefall.freebsd.org> X-Authentication-Warning: freefall.freebsd.org: gnats set sender to owner-bugmaster@FreeBSD.org using -f From: FreeBSD bugmaster To: freebsd-threads@FreeBSD.org Cc: Subject: Current problem reports assigned to freebsd-threads@FreeBSD.org 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: Mon, 20 Sep 2010 11:07:07 -0000 Note: to view an individual PR, use: http://www.freebsd.org/cgi/query-pr.cgi?pr=(number). The following is a listing of current problems submitted by FreeBSD users. These represent problem reports covering all versions including experimental development code and obsolete releases. S Tracker Resp. Description -------------------------------------------------------------------------------- o threa/149366 threads pthread_cleanup_pop never runs the configured routine o threa/148515 threads Memory / syslog strangeness in FreeBSD 8.x ( possible o threa/141721 threads rtprio(1): (id|rt)prio priority resets when new thread o threa/136345 threads Recursive read rwlocks in thread A cause deadlock with o threa/135673 threads databases/mysql50-server - MySQL query lock-ups on 7.2 o threa/133734 threads 32 bit libthr failing pthread_create() o threa/128922 threads threads hang with xorg running o threa/127225 threads bug in lib/libthr/thread/thr_init.c o threa/122923 threads 'nice' does not prevent background process from steali o threa/121336 threads lang/neko threading ok on UP, broken on SMP (FreeBSD 7 o threa/116668 threads can no longer use jdk15 with libthr on -stable SMP o threa/116181 threads /dev/io-related io access permissions are not propagat o threa/115211 threads pthread_atfork misbehaves in initial thread o threa/110636 threads [request] gdb(1): using gdb with multi thread applicat o threa/110306 threads apache 2.0 segmentation violation when calling gethost o threa/103975 threads Implicit loading/unloading of libpthread.so may crash o threa/101323 threads [patch] fork(2) in threaded programs broken. s threa/100815 threads FBSD 5.5 broke nanosleep in libc_r s threa/94467 threads send(), sendto() and sendmsg() are not correct in libc s threa/84483 threads problems with devel/nspr and -lc_r on 4.x o threa/80992 threads abort() sometimes not caught by gdb depending on threa o threa/79887 threads [patch] freopen() isn't thread-safe o threa/79683 threads svctcp_create() fails if multiple threads call at the s threa/76694 threads fork cause hang in dup()/close() function in child (-l s threa/76690 threads fork hang in child for -lc_r s threa/69020 threads pthreads library leaks _gc_mutex s threa/49087 threads Signals lost in programs linked with libc_r s threa/48856 threads Setting SIGCHLD to SIG_IGN still leaves zombies under s threa/40671 threads pthread_cancel doesn't remove thread from condition qu s threa/39922 threads [threads] [patch] Threaded applications executed with s threa/37676 threads libc_r: msgsnd(), msgrcv(), pread(), pwrite() need wra s threa/34536 threads accept() blocks other threads s threa/32295 threads [libc_r] [patch] pthread(3) dont dequeue signals s threa/30464 threads pthread mutex attributes -- pshared s threa/24632 threads libc_r delicate deviation from libc in handling SIGCHL s threa/24472 threads libc_r does not honor SO_SNDTIMEO/SO_RCVTIMEO socket o 36 problems total. From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 12:41:30 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2B2B51065695 for ; Thu, 23 Sep 2010 12:41:30 +0000 (UTC) (envelope-from alexanderchuranov@gmail.com) Received: from mail-gx0-f182.google.com (mail-gx0-f182.google.com [209.85.161.182]) by mx1.freebsd.org (Postfix) with ESMTP id DEA508FC13 for ; Thu, 23 Sep 2010 12:41:29 +0000 (UTC) Received: by gxk8 with SMTP id 8so650615gxk.13 for ; Thu, 23 Sep 2010 05:41:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:received:date:message-id :subject:from:to:content-type; bh=XIzdbBnvuHcwbKdGb5BdqFH2jtnkDlafq6zRkYg00C0=; b=x0JM5wDvp6zkl03yb1tCgNnrTMWf4KDR2sROGbW0HbluhWeDRQGatDhBMLKuMJs6+f a2OuVW0NIhz3uBMrxp6IwEl/LV0Bti4TA5Y90z1jhbFJ4XcYlo3TjN9yjV1JPW5ERSU2 Tdzgc1tGHDh7aZfBrDHzCZCplQ5/V0+8VHnqg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:date:message-id:subject:from:to:content-type; b=jEJ6Ukt2lNxQkwWdgs3dlYh2RhlU8mJNGms4y2dMs546/Ka0kxcFTRzpM19AJAeStv gOlk7R32pblxaD8qInfbSBR/QZh+EGiShfot4c/tTPfq6W0+NnaYH4/7SQTP2wuNkJL1 +S2INOYW+mD7mzZ5ATc3/o3gJTVcABgTZlH5I= MIME-Version: 1.0 Received: by 10.150.175.4 with SMTP id x4mr2401204ybe.325.1285244366983; Thu, 23 Sep 2010 05:19:26 -0700 (PDT) Received: by 10.220.81.3 with HTTP; Thu, 23 Sep 2010 05:19:26 -0700 (PDT) Date: Thu, 23 Sep 2010 16:19:26 +0400 Message-ID: From: Alexander Churanov To: freebsd-threads@freebsd.org Content-Type: text/plain; charset=ISO-8859-1 Subject: What is the status of thread process-shared synchronization? 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: Thu, 23 Sep 2010 12:41:30 -0000 Hi folks! The FreeBSD 8.1 manual pages state that pthread process-shared synchronization option is not supported, at least for some primitives. 1) Are there any plans to implement this option? 2) Is anybody working on that? 3) What are the main obstacles which prevent us from having the option implemented? I am teaching students UNIX-like systems in general and FreeBSD in particular. I've set them a task on synchronizing processes reading and writing from a shared-memory cache. But then found that in spite of PTHREAD_PROCESS_SHARED being available, it is not supported. I've endeavored to fix POSIX rwlocks by making pthread_rwlock_t a structure, but then found that POSIX semaphores do not support process-shared attribute either. 4) Do we have any synchronization primitive capable of synchronizing threads in different processes in FreeBSD? I've found a related discussion, named "A mutex for inter-process", but there is no conclusion or action items in it. Alexander Churanov From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 14:25:57 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id E640F106564A for ; Thu, 23 Sep 2010 14:25:57 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id B75DE8FC12 for ; Thu, 23 Sep 2010 14:25:57 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 66C1A46C04; Thu, 23 Sep 2010 10:25:57 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 704688A050; Thu, 23 Sep 2010 10:25:56 -0400 (EDT) From: John Baldwin To: freebsd-threads@freebsd.org Date: Thu, 23 Sep 2010 10:06:39 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009231006.40019.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 23 Sep 2010 10:25:56 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Alexander Churanov Subject: Re: What is the status of thread process-shared synchronization? 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: Thu, 23 Sep 2010 14:25:58 -0000 On Thursday, September 23, 2010 8:19:26 am Alexander Churanov wrote: > Hi folks! > > The FreeBSD 8.1 manual pages state that pthread process-shared > synchronization option is not supported, at least for some primitives. > > 1) Are there any plans to implement this option? > 2) Is anybody working on that? > 3) What are the main obstacles which prevent us from having the option > implemented? > > I am teaching students UNIX-like systems in general and FreeBSD in > particular. I've set them a task on synchronizing processes reading > and writing from a shared-memory cache. But then found that in spite > of PTHREAD_PROCESS_SHARED being available, it is not supported. I've > endeavored to fix POSIX rwlocks by making pthread_rwlock_t a > structure, but then found that POSIX semaphores do not support > process-shared attribute either. I believe POSIX semaphores in 9 do now support PTHREAD_PROCESS_SHARED. David Xu implemented it. He may be able to MFC this to 8-stable. > 4) Do we have any synchronization primitive capable of synchronizing > threads in different processes in FreeBSD? Unfortunately the various POSIX/SYSV primitives do not currently support it in 8. You could implement a shared mutex on top of umtx fairly easily I believe. umtx itself is address-space agnostic (so a umtx object can be shared among processes), and the existing pthread_mutex code can probably be borrowed directly to implement a mutex. I think the biggest obstacle for FreeBSD is changing the definition of pthread_mutex_t, etc. to be structures instead of pointers to dynamically allocated structures. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 17:40:01 2010 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 E7A6A1065693 for ; Thu, 23 Sep 2010 17:40:01 +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 AAD038FC17 for ; Thu, 23 Sep 2010 17:40:01 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8NHe1A2021308 for ; Thu, 23 Sep 2010 17:40:01 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8NHe1Ga021307; Thu, 23 Sep 2010 17:40:01 GMT (envelope-from gnats) Resent-Date: Thu, 23 Sep 2010 17:40:01 GMT Resent-Message-Id: <201009231740.o8NHe1Ga021307@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, Christopher Faylor Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0E492106566B for ; Thu, 23 Sep 2010 17:33:57 +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 F197D8FC1A for ; Thu, 23 Sep 2010 17:33:56 +0000 (UTC) Received: from www.freebsd.org (localhost [127.0.0.1]) by www.freebsd.org (8.14.3/8.14.3) with ESMTP id o8NHXu7M082525 for ; Thu, 23 Sep 2010 17:33:56 GMT (envelope-from nobody@www.freebsd.org) Received: (from nobody@localhost) by www.freebsd.org (8.14.3/8.14.3/Submit) id o8NHXuao082524; Thu, 23 Sep 2010 17:33:56 GMT (envelope-from nobody) Message-Id: <201009231733.o8NHXuao082524@www.freebsd.org> Date: Thu, 23 Sep 2010 17:33:56 GMT From: Christopher Faylor To: freebsd-gnats-submit@FreeBSD.org X-Send-Pr-Version: www-3.1 Cc: Subject: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL 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: Thu, 23 Sep 2010 17:40:02 -0000 >Number: 150889 >Category: threads >Synopsis: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-threads >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Thu Sep 23 17:40:01 UTC 2010 >Closed-Date: >Last-Modified: >Originator: Christopher Faylor >Release: 6.x - head >Organization: NetApp >Environment: FreeBSD osg-cycf64a.nane.netapp.com 7.2-RELEASE FreeBSD 7.2-RELEASE #0: Fri May 1 07:18:07 UTC 2009 root@driscoll.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC amd64 >Description: Consider the following code snippet: pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; int ret = pthread_mutex_destroy(&mutex); assert(ret == 0); This code snippet will currently always hit the assertion. This appears to be in contradiction to http://www.opengroup.org/onlinepubs/9699919799/toc.htm which states: In cases where default mutex attributes are appropriate, the macro PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes that are statically allocated. The effect shall be equivalent to dynamic initialization by a call to pthread_mutex_init() with parameter attr specified as NULL, except that no error checks are performed. >How-To-Repeat: See description. >Fix: It seems like the simple fix is to change the ret = EINVAL in lib/libthr/thread/thr_mutex.c to a ret = 0 >Release-Note: >Audit-Trail: >Unformatted: From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 18:14:58 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A869E1065672; Thu, 23 Sep 2010 18:14:58 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 62B218FC23; Thu, 23 Sep 2010 18:14:58 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 1339946B49; Thu, 23 Sep 2010 14:14:52 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id D65E78A04E; Thu, 23 Sep 2010 14:14:50 -0400 (EDT) From: John Baldwin To: freebsd-threads@freebsd.org Date: Thu, 23 Sep 2010 14:14:50 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201009231733.o8NHXuao082524@www.freebsd.org> In-Reply-To: <201009231733.o8NHXuao082524@www.freebsd.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-15" Content-Transfer-Encoding: 7bit Message-Id: <201009231414.50271.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 23 Sep 2010 14:14:50 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-gnats-submit@freebsd.org, Christopher Faylor Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL 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: Thu, 23 Sep 2010 18:14:58 -0000 On Thursday, September 23, 2010 1:33:56 pm Christopher Faylor wrote: > > >Number: 150889 > >Category: threads > >Synopsis: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL > >Confidential: no > >Severity: non-critical > >Priority: low > >Responsible: freebsd-threads > >State: open > >Quarter: > >Keywords: > >Date-Required: > >Class: sw-bug > >Submitter-Id: current-users > >Arrival-Date: Thu Sep 23 17:40:01 UTC 2010 > >Closed-Date: > >Last-Modified: > >Originator: Christopher Faylor > >Release: 6.x - head > >Organization: > NetApp > >Environment: > FreeBSD osg-cycf64a.nane.netapp.com 7.2-RELEASE FreeBSD 7.2-RELEASE #0: Fri May 1 07:18:07 UTC 2009 root@driscoll.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC amd64 > >Description: > Consider the following code snippet: > > pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; > int ret = pthread_mutex_destroy(&mutex); > assert(ret == 0); > > This code snippet will currently always hit the assertion. > > This appears to be in contradiction to > > http://www.opengroup.org/onlinepubs/9699919799/toc.htm > > which states: > > In cases where default mutex attributes are appropriate, the macro > PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes that are statically > allocated. The effect shall be equivalent to dynamic initialization by a call to > pthread_mutex_init() with parameter attr specified as NULL, except that no error > checks are performed. I suppose that is true, but I think this is also probably buggy code if you are doing this. The use case for PTHREAD_MUTEX_INITALIZER is static initialization of static objects to ease adding locking to C libraries where constructors are not easy. Specifically, to avoid prefixing every pthread_mutex_lock() with a pthread_once() call to initialize the mutex. (See the example given in the RATIONALE section in the pthread_mutex_init() page at the link above where it talks about static initialization.) Such mutexes are generally never destroyed (their lifetime is the same as the containing executable or shared library (since presumably they could vanish as part of a dlclose())). I think it is not provided so that you can initialize dynamically allocated mutexes at runtime via: struct foo { ... pthread_mutex_t m; } f = malloc(sizeof(foo); f->m = PTHREAD_MUTEX_INITIALIZER; Those sorts of locks should be initialized via pthread_mutex_init() instead. All that said, we do use a rather gross hack that is similar in stdio. We create a template FILE object like this: static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; and use structure assignment to initialize new FILE objects that are allocated dynamically: g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); p = (FILE *)ALIGN(g + 1); while (--n >= 0) *p++ = empty; Of course, this is internal to the implementation of stdio and isn't necessarily portable. In this case stdio is probably doing it this way to avoid the dance of invoking pthread_mutex_init() in the non-threaded case to avoid "poisoning" the pthread_mutex_init() weak symbol. That problem is unique to libc's implementation, however. I would not do this in portable code. :) The one possibly valid reason I can see for pthread_mutex_destroy() to operate on a statically initialized mutex is to let a library destroy a mutex due to dlclose() unloading the library. However, to pull that off the library would need to install an atexit() hook or similar to actually invoke pthread_mutex_destroy(). That requires some sort of initialization code to invoke atexit() at which point you might as well call pthread_mutex_init() to initialize the mutex at the same time as calling atexit(). -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 18:20:03 2010 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 96D021065674 for ; Thu, 23 Sep 2010 18:20: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 6B82E8FC17 for ; Thu, 23 Sep 2010 18:20:03 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8NIK3IE063461 for ; Thu, 23 Sep 2010 18:20:03 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8NIK3fA063459; Thu, 23 Sep 2010 18:20:03 GMT (envelope-from gnats) Date: Thu, 23 Sep 2010 18:20:03 GMT Message-Id: <201009231820.o8NIK3fA063459@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: John Baldwin Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: John Baldwin List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Sep 2010 18:20:03 -0000 The following reply was made to PR threads/150889; it has been noted by GNATS. From: John Baldwin To: freebsd-threads@freebsd.org Cc: Christopher Faylor , freebsd-gnats-submit@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL Date: Thu, 23 Sep 2010 14:14:50 -0400 On Thursday, September 23, 2010 1:33:56 pm Christopher Faylor wrote: > > >Number: 150889 > >Category: threads > >Synopsis: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL > >Confidential: no > >Severity: non-critical > >Priority: low > >Responsible: freebsd-threads > >State: open > >Quarter: > >Keywords: > >Date-Required: > >Class: sw-bug > >Submitter-Id: current-users > >Arrival-Date: Thu Sep 23 17:40:01 UTC 2010 > >Closed-Date: > >Last-Modified: > >Originator: Christopher Faylor > >Release: 6.x - head > >Organization: > NetApp > >Environment: > FreeBSD osg-cycf64a.nane.netapp.com 7.2-RELEASE FreeBSD 7.2-RELEASE #0: Fri May 1 07:18:07 UTC 2009 root@driscoll.cse.buffalo.edu:/usr/obj/usr/src/sys/GENERIC amd64 > >Description: > Consider the following code snippet: > > pthread_mutex_t mutex = PTHREAD_MUTEX_INITIALIZER; > int ret = pthread_mutex_destroy(&mutex); > assert(ret == 0); > > This code snippet will currently always hit the assertion. > > This appears to be in contradiction to > > http://www.opengroup.org/onlinepubs/9699919799/toc.htm > > which states: > > In cases where default mutex attributes are appropriate, the macro > PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes that are statically > allocated. The effect shall be equivalent to dynamic initialization by a call to > pthread_mutex_init() with parameter attr specified as NULL, except that no error > checks are performed. I suppose that is true, but I think this is also probably buggy code if you are doing this. The use case for PTHREAD_MUTEX_INITALIZER is static initialization of static objects to ease adding locking to C libraries where constructors are not easy. Specifically, to avoid prefixing every pthread_mutex_lock() with a pthread_once() call to initialize the mutex. (See the example given in the RATIONALE section in the pthread_mutex_init() page at the link above where it talks about static initialization.) Such mutexes are generally never destroyed (their lifetime is the same as the containing executable or shared library (since presumably they could vanish as part of a dlclose())). I think it is not provided so that you can initialize dynamically allocated mutexes at runtime via: struct foo { ... pthread_mutex_t m; } f = malloc(sizeof(foo); f->m = PTHREAD_MUTEX_INITIALIZER; Those sorts of locks should be initialized via pthread_mutex_init() instead. All that said, we do use a rather gross hack that is similar in stdio. We create a template FILE object like this: static FILE empty = { ._fl_mutex = PTHREAD_MUTEX_INITIALIZER }; and use structure assignment to initialize new FILE objects that are allocated dynamically: g = (struct glue *)malloc(sizeof(*g) + ALIGNBYTES + n * sizeof(FILE)); p = (FILE *)ALIGN(g + 1); while (--n >= 0) *p++ = empty; Of course, this is internal to the implementation of stdio and isn't necessarily portable. In this case stdio is probably doing it this way to avoid the dance of invoking pthread_mutex_init() in the non-threaded case to avoid "poisoning" the pthread_mutex_init() weak symbol. That problem is unique to libc's implementation, however. I would not do this in portable code. :) The one possibly valid reason I can see for pthread_mutex_destroy() to operate on a statically initialized mutex is to let a library destroy a mutex due to dlclose() unloading the library. However, to pull that off the library would need to install an atexit() hook or similar to actually invoke pthread_mutex_destroy(). That requires some sort of initialization code to invoke atexit() at which point you might as well call pthread_mutex_init() to initialize the mutex at the same time as calling atexit(). -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 20:10:02 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 5F5831065670; Thu, 23 Sep 2010 20:10:02 +0000 (UTC) (envelope-from Christopher.Faylor@netapp.com) Received: from mx2.netapp.com (mx2.netapp.com [216.240.18.37]) by mx1.freebsd.org (Postfix) with ESMTP id 3C9F18FC13; Thu, 23 Sep 2010 20:10:02 +0000 (UTC) X-IronPort-AV: E=Sophos;i="4.57,224,1283756400"; d="scan'208";a="456836333" Received: from smtp2.corp.netapp.com ([10.57.159.114]) by mx2-out.netapp.com with ESMTP; 23 Sep 2010 12:41:54 -0700 Received: from sacrsexc2-prd.hq.netapp.com (sacrsexc2-prd.hq.netapp.com [10.99.115.28]) by smtp2.corp.netapp.com (8.13.1/8.13.1/NTAP-1.6) with ESMTP id o8NJfrsI000104; Thu, 23 Sep 2010 12:41:54 -0700 (PDT) Received: from rtprsexc2-prd.hq.netapp.com ([10.100.161.115]) by sacrsexc2-prd.hq.netapp.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 23 Sep 2010 12:41:54 -0700 Received: from RTPMVEXC1-PRD.hq.netapp.com ([10.100.161.112]) by rtprsexc2-prd.hq.netapp.com with Microsoft SMTPSVC(6.0.3790.3959); Thu, 23 Sep 2010 15:41:52 -0400 Received: from 10.30.34.57 ([10.30.34.57]) by RTPMVEXC1-PRD.hq.netapp.com ([10.100.161.119]) with Microsoft Exchange Server HTTP-DAV ; Thu, 23 Sep 2010 19:41:51 +0000 Received: from trixie by RTPMVEXC1-PRD.hq.netapp.com; 23 Sep 2010 15:41:51 -0400 From: Christopher Faylor To: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org In-Reply-To: <201009231414.50271.jhb@freebsd.org> References: <201009231733.o8NHXuao082524@www.freebsd.org> <201009231414.50271.jhb@freebsd.org> Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable Date: Thu, 23 Sep 2010 15:41:51 -0400 Message-ID: <1285270911.11313.30.camel@trixie.casa.cgf.cx> Mime-Version: 1.0 X-Mailer: Evolution 2.28.2 X-OriginalArrivalTime: 23 Sep 2010 19:41:52.0592 (UTC) FILETIME=[5F189500:01CB5B57] Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL 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: Thu, 23 Sep 2010 20:10:02 -0000 On Thu, 2010-09-23 at 14:14 -0400, John Baldwin wrote: > I suppose that is true, but I think this is also probably buggy code if y= ou > are doing this. The use case for PTHREAD_MUTEX_INITALIZER is static > initialization of static objects to ease adding locking to C libraries wh= ere > constructors are not easy. Specifically, to avoid prefixing every > pthread_mutex_lock() with a pthread_once() call to initialize the mutex. > (See the example given in the RATIONALE section in the pthread_mutex_init= () > page at the link above where it talks about static initialization.) Such > mutexes are generally never destroyed (their lifetime is the same as the > containing executable or shared library (since presumably they could vani= sh > as part of a dlclose())). I think it is not provided so that you can > initialize dynamically allocated mutexes at runtime via: >=20 > struct foo { > ... > pthread_mutex_t m; > } >=20 > f =3D malloc(sizeof(foo); > f->m =3D PTHREAD_MUTEX_INITIALIZER; >=20 > Those sorts of locks should be initialized via pthread_mutex_init() inste= ad. >... >=20 > The one possibly valid reason I can see for pthread_mutex_destroy() to > operate on a statically initialized mutex is to let a library destroy a > mutex due to dlclose() unloading the library. However, to pull that off = the > library would need to install an atexit() hook or similar to actually inv= oke > pthread_mutex_destroy(). That requires some sort of initialization code = to > invoke atexit() at which point you might as well call pthread_mutex_init(= ) > to initialize the mutex at the same time as calling atexit(). I don't see how this represents buggy code. It should be possible to destroy a mutex which is allocated statically. Currently, if a mutex is assigned to PTHREAD_MUTEX_INITIALIZER and then used once, it can be successfully destroyed. It is only receive an EINVAL when there has been no intervening call to any mutex function. I don't think that a PTHREAD_MUTEX_INITIALIZER using program should have to check for that. However, regardless, this is still a bug in pthread_mutex_destroy right? From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 20:20:04 2010 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 A4A0A106566B for ; Thu, 23 Sep 2010 20:20:04 +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 793C78FC19 for ; Thu, 23 Sep 2010 20:20:04 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8NKK4tj087766 for ; Thu, 23 Sep 2010 20:20:04 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8NKK4mg087765; Thu, 23 Sep 2010 20:20:04 GMT (envelope-from gnats) Date: Thu, 23 Sep 2010 20:20:04 GMT Message-Id: <201009232020.o8NKK4mg087765@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Christopher Faylor Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Christopher Faylor List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Sep 2010 20:20:04 -0000 The following reply was made to PR threads/150889; it has been noted by GNATS. From: Christopher Faylor To: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL Date: Thu, 23 Sep 2010 15:41:51 -0400 On Thu, 2010-09-23 at 14:14 -0400, John Baldwin wrote: > I suppose that is true, but I think this is also probably buggy code if y= ou > are doing this. The use case for PTHREAD_MUTEX_INITALIZER is static > initialization of static objects to ease adding locking to C libraries wh= ere > constructors are not easy. Specifically, to avoid prefixing every > pthread_mutex_lock() with a pthread_once() call to initialize the mutex. > (See the example given in the RATIONALE section in the pthread_mutex_init= () > page at the link above where it talks about static initialization.) Such > mutexes are generally never destroyed (their lifetime is the same as the > containing executable or shared library (since presumably they could vani= sh > as part of a dlclose())). I think it is not provided so that you can > initialize dynamically allocated mutexes at runtime via: >=20 > struct foo { > ... > pthread_mutex_t m; > } >=20 > f =3D malloc(sizeof(foo); > f->m =3D PTHREAD_MUTEX_INITIALIZER; >=20 > Those sorts of locks should be initialized via pthread_mutex_init() inste= ad. >... >=20 > The one possibly valid reason I can see for pthread_mutex_destroy() to > operate on a statically initialized mutex is to let a library destroy a > mutex due to dlclose() unloading the library. However, to pull that off = the > library would need to install an atexit() hook or similar to actually inv= oke > pthread_mutex_destroy(). That requires some sort of initialization code = to > invoke atexit() at which point you might as well call pthread_mutex_init(= ) > to initialize the mutex at the same time as calling atexit(). I don't see how this represents buggy code. It should be possible to destroy a mutex which is allocated statically. Currently, if a mutex is assigned to PTHREAD_MUTEX_INITIALIZER and then used once, it can be successfully destroyed. It is only receive an EINVAL when there has been no intervening call to any mutex function. I don't think that a PTHREAD_MUTEX_INITIALIZER using program should have to check for that. However, regardless, this is still a bug in pthread_mutex_destroy right? From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 21:07:47 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 96692106564A; Thu, 23 Sep 2010 21:07:47 +0000 (UTC) (envelope-from jilles@stack.nl) Received: from mx1.stack.nl (relay04.stack.nl [IPv6:2001:610:1108:5010::107]) by mx1.freebsd.org (Postfix) with ESMTP id 5A4948FC1A; Thu, 23 Sep 2010 21:07:47 +0000 (UTC) Received: from turtle.stack.nl (turtle.stack.nl [IPv6:2001:610:1108:5010::132]) by mx1.stack.nl (Postfix) with ESMTP id 5C8281DD415; Thu, 23 Sep 2010 23:07:46 +0200 (CEST) Received: by turtle.stack.nl (Postfix, from userid 1677) id 46A6D172E5; Thu, 23 Sep 2010 23:07:46 +0200 (CEST) Date: Thu, 23 Sep 2010 23:07:46 +0200 From: Jilles Tjoelker To: Christopher Faylor Message-ID: <20100923210746.GA44173@stack.nl> References: <201009231733.o8NHXuao082524@www.freebsd.org> <201009231414.50271.jhb@freebsd.org> <1285270911.11313.30.camel@trixie.casa.cgf.cx> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1285270911.11313.30.camel@trixie.casa.cgf.cx> User-Agent: Mutt/1.5.20 (2009-06-14) Cc: freebsd-gnats-submit@freebsd.org, freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL 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: Thu, 23 Sep 2010 21:07:47 -0000 On Thu, Sep 23, 2010 at 03:41:51PM -0400, Christopher Faylor wrote: > I don't see how this represents buggy code. It should be possible to > destroy a mutex which is allocated statically. Currently, if a mutex is > assigned to PTHREAD_MUTEX_INITIALIZER and then used once, it can be > successfully destroyed. It is only receive an EINVAL when there has > been no intervening call to any mutex function. I don't think that a > PTHREAD_MUTEX_INITIALIZER using program should have to check for that. One may want to destroy a mutex to help memory leak checkers and detect bugs, and then this is indeed a problem. > However, regardless, this is still a bug in pthread_mutex_destroy right? It is inconsistent at best. It seems best to make the proposed change. This will allow pthread_mutex_destroy() on a destroyed mutex to succeed (which used to return EINVAL), but pthread_mutex_lock() already succeeded as well (initializing the mutex in the process). If/when pthread_mutex_t is made a struct, this can be revisited, and most likely the destroyed and PTHREAD_MUTEX_INITIALIZER states will be different (PTHREAD_MUTEX_INITIALIZER will likely be a normal state that does not need special initialization to use). -- Jilles Tjoelker From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 21:10:04 2010 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 74048106564A for ; Thu, 23 Sep 2010 21:10:04 +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 62F558FC15 for ; Thu, 23 Sep 2010 21:10:04 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8NLA4Oh039570 for ; Thu, 23 Sep 2010 21:10:04 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8NLA4io039569; Thu, 23 Sep 2010 21:10:04 GMT (envelope-from gnats) Date: Thu, 23 Sep 2010 21:10:04 GMT Message-Id: <201009232110.o8NLA4io039569@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Jilles Tjoelker Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Jilles Tjoelker List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Sep 2010 21:10:04 -0000 The following reply was made to PR threads/150889; it has been noted by GNATS. From: Jilles Tjoelker To: Christopher Faylor Cc: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL Date: Thu, 23 Sep 2010 23:07:46 +0200 On Thu, Sep 23, 2010 at 03:41:51PM -0400, Christopher Faylor wrote: > I don't see how this represents buggy code. It should be possible to > destroy a mutex which is allocated statically. Currently, if a mutex is > assigned to PTHREAD_MUTEX_INITIALIZER and then used once, it can be > successfully destroyed. It is only receive an EINVAL when there has > been no intervening call to any mutex function. I don't think that a > PTHREAD_MUTEX_INITIALIZER using program should have to check for that. One may want to destroy a mutex to help memory leak checkers and detect bugs, and then this is indeed a problem. > However, regardless, this is still a bug in pthread_mutex_destroy right? It is inconsistent at best. It seems best to make the proposed change. This will allow pthread_mutex_destroy() on a destroyed mutex to succeed (which used to return EINVAL), but pthread_mutex_lock() already succeeded as well (initializing the mutex in the process). If/when pthread_mutex_t is made a struct, this can be revisited, and most likely the destroyed and PTHREAD_MUTEX_INITIALIZER states will be different (PTHREAD_MUTEX_INITIALIZER will likely be a normal state that does not need special initialization to use). -- Jilles Tjoelker From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 21:41:12 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 32170106564A; Thu, 23 Sep 2010 21:41:12 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 016218FC18; Thu, 23 Sep 2010 21:41:12 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id B0DB246B09; Thu, 23 Sep 2010 17:41:11 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id E38148A03C; Thu, 23 Sep 2010 17:41:09 -0400 (EDT) From: John Baldwin To: freebsd-threads@freebsd.org Date: Thu, 23 Sep 2010 17:41:07 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201009231733.o8NHXuao082524@www.freebsd.org> <1285270911.11313.30.camel@trixie.casa.cgf.cx> <20100923210746.GA44173@stack.nl> In-Reply-To: <20100923210746.GA44173@stack.nl> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009231741.07962.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Thu, 23 Sep 2010 17:41:09 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: freebsd-gnats-submit@freebsd.org, Christopher Faylor Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL 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: Thu, 23 Sep 2010 21:41:12 -0000 On Thursday, September 23, 2010 5:07:46 pm Jilles Tjoelker wrote: > On Thu, Sep 23, 2010 at 03:41:51PM -0400, Christopher Faylor wrote: > > I don't see how this represents buggy code. It should be possible to > > destroy a mutex which is allocated statically. Currently, if a mutex is > > assigned to PTHREAD_MUTEX_INITIALIZER and then used once, it can be > > successfully destroyed. It is only receive an EINVAL when there has > > been no intervening call to any mutex function. I don't think that a > > PTHREAD_MUTEX_INITIALIZER using program should have to check for that. > > One may want to destroy a mutex to help memory leak checkers and detect > bugs, and then this is indeed a problem. > > > However, regardless, this is still a bug in pthread_mutex_destroy right? > > It is inconsistent at best. > > It seems best to make the proposed change. This will allow > pthread_mutex_destroy() on a destroyed mutex to succeed (which used to > return EINVAL), but pthread_mutex_lock() already succeeded as well > (initializing the mutex in the process). Hmm, I think that POSIX actually require these to fail (ideally with EBUSY rather than EINVAL). Presumably pthread_mutex_destroy() needs to mark mutexes with a value different from PTHREAD_MUTEX_INITIALIZER when it destroys them (similar to MTX_DEAD in the kernel). This is actually very useful behavior for catching bugs and we should catch that. We probably should make pthread_mutex_destroy() not fail but do whatever is sensible for a mutex initialized statically in that case however. > If/when pthread_mutex_t is made a struct, this can be revisited, and > most likely the destroyed and PTHREAD_MUTEX_INITIALIZER states will be > different (PTHREAD_MUTEX_INITIALIZER will likely be a normal state that > does not need special initialization to use). I would argue that they should already be different states. I'm surprised our pthread_mutex_destroy() is that broken. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 21:50:03 2010 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 0266E106566C for ; Thu, 23 Sep 2010 21:50: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 CB1F58FC19 for ; Thu, 23 Sep 2010 21:50:02 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8NLo2g3079385 for ; Thu, 23 Sep 2010 21:50:02 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8NLo2f5079384; Thu, 23 Sep 2010 21:50:02 GMT (envelope-from gnats) Date: Thu, 23 Sep 2010 21:50:02 GMT Message-Id: <201009232150.o8NLo2f5079384@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: John Baldwin Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: John Baldwin List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Sep 2010 21:50:03 -0000 The following reply was made to PR threads/150889; it has been noted by GNATS. From: John Baldwin To: freebsd-threads@freebsd.org Cc: Jilles Tjoelker , Christopher Faylor , freebsd-gnats-submit@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL Date: Thu, 23 Sep 2010 17:41:07 -0400 On Thursday, September 23, 2010 5:07:46 pm Jilles Tjoelker wrote: > On Thu, Sep 23, 2010 at 03:41:51PM -0400, Christopher Faylor wrote: > > I don't see how this represents buggy code. It should be possible to > > destroy a mutex which is allocated statically. Currently, if a mutex is > > assigned to PTHREAD_MUTEX_INITIALIZER and then used once, it can be > > successfully destroyed. It is only receive an EINVAL when there has > > been no intervening call to any mutex function. I don't think that a > > PTHREAD_MUTEX_INITIALIZER using program should have to check for that. > > One may want to destroy a mutex to help memory leak checkers and detect > bugs, and then this is indeed a problem. > > > However, regardless, this is still a bug in pthread_mutex_destroy right? > > It is inconsistent at best. > > It seems best to make the proposed change. This will allow > pthread_mutex_destroy() on a destroyed mutex to succeed (which used to > return EINVAL), but pthread_mutex_lock() already succeeded as well > (initializing the mutex in the process). Hmm, I think that POSIX actually require these to fail (ideally with EBUSY rather than EINVAL). Presumably pthread_mutex_destroy() needs to mark mutexes with a value different from PTHREAD_MUTEX_INITIALIZER when it destroys them (similar to MTX_DEAD in the kernel). This is actually very useful behavior for catching bugs and we should catch that. We probably should make pthread_mutex_destroy() not fail but do whatever is sensible for a mutex initialized statically in that case however. > If/when pthread_mutex_t is made a struct, this can be revisited, and > most likely the destroyed and PTHREAD_MUTEX_INITIALIZER states will be > different (PTHREAD_MUTEX_INITIALIZER will likely be a normal state that > does not need special initialization to use). I would argue that they should already be different states. I'm surprised our pthread_mutex_destroy() is that broken. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 22:20:03 2010 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 CBF56106564A for ; Thu, 23 Sep 2010 22:20: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 A0B868FC12 for ; Thu, 23 Sep 2010 22:20:03 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8NMK3G1011640 for ; Thu, 23 Sep 2010 22:20:03 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8NMK3fX011639; Thu, 23 Sep 2010 22:20:03 GMT (envelope-from gnats) Date: Thu, 23 Sep 2010 22:20:03 GMT Message-Id: <201009232220.o8NMK3fX011639@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Jung-uk Kim Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Jung-uk Kim List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Sep 2010 22:20:03 -0000 The following reply was made to PR threads/150889; it has been noted by GNATS. From: Jung-uk Kim To: bug-followup@FreeBSD.org, cgf@netapp.com Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL Date: Thu, 23 Sep 2010 18:12:02 -0400 I was horribly bitten by the incompatibility when I ported IcedTea6 Java plugin for Linux. Please see my comments in the patch: http://www.freebsd.org/cgi/cvsweb.cgi/ports/java/openjdk6/files/icedtea.patch?rev=1.2;content-type=text%2Fplain FYI... Jung-uk Kim From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 22:58:42 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id CFB9B106564A; Thu, 23 Sep 2010 22:58:42 +0000 (UTC) (envelope-from deischen@freebsd.org) Received: from mail.netplex.net (mail.netplex.net [204.213.176.10]) by mx1.freebsd.org (Postfix) with ESMTP id 8C0178FC13; Thu, 23 Sep 2010 22:58:42 +0000 (UTC) Received: from sea.ntplx.net (sea.ntplx.net [204.213.176.11]) by mail.netplex.net (8.14.4/8.14.4/NETPLEX) with ESMTP id o8NMitmV003705; Thu, 23 Sep 2010 18:44:56 -0400 (EDT) X-Virus-Scanned: by AMaViS and Clam AntiVirus (mail.netplex.net) X-Greylist: Message whitelisted by DRAC access database, not delayed by milter-greylist-4.2.2 (mail.netplex.net [204.213.176.10]); Thu, 23 Sep 2010 18:44:56 -0400 (EDT) Date: Thu, 23 Sep 2010 18:44:55 -0400 (EDT) From: Daniel Eischen X-X-Sender: eischen@sea.ntplx.net To: Jung-uk Kim In-Reply-To: <201009232220.o8NMK3fX011639@freefall.freebsd.org> Message-ID: References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Daniel Eischen List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Sep 2010 22:58:42 -0000 On Thu, 23 Sep 2010, Jung-uk Kim wrote: > The following reply was made to PR threads/150889; it has been noted by GNATS. > > From: Jung-uk Kim > To: bug-followup@FreeBSD.org, > cgf@netapp.com > Cc: > Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL > Date: Thu, 23 Sep 2010 18:12:02 -0400 > > I was horribly bitten by the incompatibility when I ported IcedTea6 > Java plugin for Linux. Please see my comments in the patch: > > http://www.freebsd.org/cgi/cvsweb.cgi/ports/java/openjdk6/files/icedtea.patch?rev=1.2;content-type=text%2Fplain You shouldn't have to call pthread_mutex_init() on a mutex initialized with PTHREAD_MUTEX_INITIALIZER. Our implementation should auto initialize the mutex when it is first used; if it doesn't, I think that is a bug. You _do have_ to lock the mutex before calling a condition wait, however. This is a POSIX requirement. -- DE From owner-freebsd-threads@FreeBSD.ORG Thu Sep 23 23:10:17 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 429BA106564A; Thu, 23 Sep 2010 23:10:17 +0000 (UTC) (envelope-from kabaev@gmail.com) Received: from mail-qy0-f175.google.com (mail-qy0-f175.google.com [209.85.216.175]) by mx1.freebsd.org (Postfix) with ESMTP id C2D9F8FC0C; Thu, 23 Sep 2010 23:10:16 +0000 (UTC) Received: by mail-qy0-f175.google.com with SMTP id 31so419173qyk.13 for ; Thu, 23 Sep 2010 16:10:16 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:in-reply-to:references:x-mailer:mime-version :content-type; bh=eMmEKx1hv9l9DHwkUjBJdeDESrq1qu6e+1TaRO1tMt0=; b=O4xGSBJxU2YKYhRJ6ei//vFQGklGTR74CpHFTnPzlVAFabQ2CqV7x6upQ2wKtcg5CG R5iK1cOQkOz9zPSIHMO7sXMr2EAAV1qbLmZu+mUd2SiyqZGmEPTaW5HP1qDREKYeYkan Ul5wBtL+9i6OAR6kXe8/LcrBdEjM/e80YeHMo= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:in-reply-to:references:x-mailer :mime-version:content-type; b=tkzCKgWQzvOdhq0lERJUkLdZeeJ7WTUeBYDWws9SI64DRh37vyyvohQjwN6KCFcoLg eU6Er9ghE7UbLBLppTGVtDA2zxWijewHq1F1ZVl8SliXZs5yNuhDIvr/fGN4hET+67qv kVuFHMoJA1tm9PmSUOAj67jYrVIz4xqe1eqM4= Received: by 10.229.51.219 with SMTP id e27mr1868243qcg.250.1285281545585; Thu, 23 Sep 2010 15:39:05 -0700 (PDT) Received: from kan.dnsalias.net (c-24-63-226-98.hsd1.ma.comcast.net [24.63.226.98]) by mx.google.com with ESMTPS id r36sm1496714qcs.15.2010.09.23.15.39.03 (version=SSLv3 cipher=RC4-MD5); Thu, 23 Sep 2010 15:39:04 -0700 (PDT) Date: Thu, 23 Sep 2010 18:38:44 -0400 From: Alexander Kabaev To: Jung-uk Kim Message-ID: <20100923183844.1823928a@kan.dnsalias.net> In-Reply-To: <201009232220.o8NMK3fX011639@freefall.freebsd.org> References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> X-Mailer: Claws Mail 3.7.6 (GTK+ 2.20.1; amd64-portbld-freebsd9.0) Mime-Version: 1.0 Content-Type: multipart/signed; micalg=PGP-SHA1; boundary="Sig_/GalQ4ZHqC6OakA/jFCU9XJb"; protocol="application/pgp-signature" Cc: freebsd-threads@FreeBSD.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL 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: Thu, 23 Sep 2010 23:10:17 -0000 --Sig_/GalQ4ZHqC6OakA/jFCU9XJb Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable On Thu, 23 Sep 2010 22:20:03 GMT Jung-uk Kim wrote: > The following reply was made to PR threads/150889; it has been noted > by GNATS. >=20 > From: Jung-uk Kim > To: bug-followup@FreeBSD.org, > cgf@netapp.com > Cc: =20 > Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + > pthread_mutex_destroy() =3D=3D EINVAL Date: Thu, 23 Sep 2010 18:12:02 > -0400 >=20 > I was horribly bitten by the incompatibility when I ported IcedTea6=20 > Java plugin for Linux. Please see my comments in the patch: > =20 > http://www.freebsd.org/cgi/cvsweb.cgi/ports/java/openjdk6/files/icedtea.= patch?rev=3D1.2;content-type=3Dtext%2Fplain > =20 " The pthread_cond_timedwait() and pthread_cond_wait() functions shall block on a condition variable. They shall be called with mutex locked by the calling thread or undefined behavior results. " NPTL is being non-compliant here. --=20 Alexander Kabaev --Sig_/GalQ4ZHqC6OakA/jFCU9XJb Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (FreeBSD) iD8DBQFMm9cGQ6z1jMm+XZYRApNWAJ47Mb/lfZcXf9tjVpiAQEcKbQjLwACg1qHd B6wyK2h13rLhoszc9IdIORU= =H+Ll -----END PGP SIGNATURE----- --Sig_/GalQ4ZHqC6OakA/jFCU9XJb-- From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 03:48:55 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 4E0FC106566B; Fri, 24 Sep 2010 03:48:55 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: Daniel Eischen Date: Thu, 23 Sep 2010 23:48:40 -0400 User-Agent: KMail/1.6.2 References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> In-Reply-To: MIME-Version: 1.0 Content-Disposition: inline Message-Id: <201009232348.45201.jkim@FreeBSD.org> Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Cc: freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 03:48:56 -0000 On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > You shouldn't have to call pthread_mutex_init() on a mutex > initialized with PTHREAD_MUTEX_INITIALIZER. Our implementation > should auto initialize the mutex when it is first used; if it > doesn't, I think that is a bug. Ah, I see. I verified that libthr does it correctly. However, that's a hack and it is far from real static allocation although it should work pretty well in reality, IMHO. More over, it will have a side-effect, i.e., any destroyed mutex may be resurrected if it is used again. POSIX seems to say it should return EINVAL when it happens. :-( > You _do have_ to lock the mutex before calling a condition > wait, however. This is a POSIX requirement. Yes, understood. Thanks, Jung-uk Kim From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 08:59:40 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id BE1BD1065672; Fri, 24 Sep 2010 08:59:40 +0000 (UTC) (envelope-from tmueller@sysgo.com) Received: from mail.sysgo.com (mail.sysgo.com [195.145.229.155]) by mx1.freebsd.org (Postfix) with ESMTP id 7E75B8FC17; Fri, 24 Sep 2010 08:59:40 +0000 (UTC) Received: from lantia.sysgo.com (unknown [172.22.2.7]) by mail.sysgo.com (Postfix) with ESMTP id 36D3414119; Fri, 24 Sep 2010 10:43:48 +0200 (CEST) Received: by lantia.sysgo.com (Postfix, from userid 113) id 135D05A295; Fri, 24 Sep 2010 10:43:48 +0200 (CEST) Received: from tmu.ulm.sysgo.com (tmu.ulm.sysgo.com [172.30.3.10]) by lantia.sysgo.com (Postfix) with ESMTP id E59C95A281; Fri, 24 Sep 2010 10:43:47 +0200 (CEST) Date: Fri, 24 Sep 2010 10:38:05 +0200 From: Thomas Mueller To: John Baldwin Message-ID: <20100924103805.75c16735@tmu.ulm.sysgo.com> In-Reply-To: <201009231414.50271.jhb@freebsd.org> References: <201009231733.o8NHXuao082524@www.freebsd.org> <201009231414.50271.jhb@freebsd.org> Organization: SYSGO AG X-Mailer: Claws Mail 3.7.6 (GTK+ 2.20.1; amd64-portbld-freebsd8.1) Mime-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Cc: freebsd-gnats-submit@freebsd.org, Christopher Faylor , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL 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: Fri, 24 Sep 2010 08:59:40 -0000 On Thu, 23 Sep 2010 14:14:50 -0400, John Baldwin wrote: > On Thursday, September 23, 2010 1:33:56 pm Christopher Faylor wrote: > > In cases where default mutex attributes are appropriate, the macro > > PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes that are statically > > allocated. The effect shall be equivalent to dynamic initialization by a call to > > pthread_mutex_init() with parameter attr specified as NULL, except that no error > > checks are performed. > > I suppose that is true, but I think this is also probably buggy code if you > are doing this. The use case for PTHREAD_MUTEX_INITALIZER is static > initialization of static objects to ease adding locking to C libraries where > constructors are not easy. Note that future standard text might drop the "statically allocated" part in the pthread_mutex_init() description, see also http://www.austingroupbugs.net/view.php?id=70 -- Thomas Mueller From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 09:00:15 2010 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 C76D91065693 for ; Fri, 24 Sep 2010 09:00:15 +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 B6ED48FC1A for ; Fri, 24 Sep 2010 09:00:15 +0000 (UTC) Received: from freefall.freebsd.org (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8O90FxT015182 for ; Fri, 24 Sep 2010 09:00:15 GMT (envelope-from gnats@freefall.freebsd.org) Received: (from gnats@localhost) by freefall.freebsd.org (8.14.4/8.14.4/Submit) id o8O90FYm015166; Fri, 24 Sep 2010 09:00:15 GMT (envelope-from gnats) Date: Fri, 24 Sep 2010 09:00:15 GMT Message-Id: <201009240900.o8O90FYm015166@freefall.freebsd.org> To: freebsd-threads@FreeBSD.org From: Thomas Mueller Cc: Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL X-BeenThere: freebsd-threads@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Thomas Mueller List-Id: Threading on FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 24 Sep 2010 09:00:15 -0000 The following reply was made to PR threads/150889; it has been noted by GNATS. From: Thomas Mueller To: John Baldwin Cc: freebsd-threads@freebsd.org, freebsd-gnats-submit@freebsd.org, Christopher Faylor Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy() == EINVAL Date: Fri, 24 Sep 2010 10:38:05 +0200 On Thu, 23 Sep 2010 14:14:50 -0400, John Baldwin wrote: > On Thursday, September 23, 2010 1:33:56 pm Christopher Faylor wrote: > > In cases where default mutex attributes are appropriate, the macro > > PTHREAD_MUTEX_INITIALIZER can be used to initialize mutexes that are statically > > allocated. The effect shall be equivalent to dynamic initialization by a call to > > pthread_mutex_init() with parameter attr specified as NULL, except that no error > > checks are performed. > > I suppose that is true, but I think this is also probably buggy code if you > are doing this. The use case for PTHREAD_MUTEX_INITALIZER is static > initialization of static objects to ease adding locking to C libraries where > constructors are not easy. Note that future standard text might drop the "statically allocated" part in the pthread_mutex_init() description, see also http://www.austingroupbugs.net/view.php?id=70 -- Thomas Mueller From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 13:28:53 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id D3CCA106564A; Fri, 24 Sep 2010 13:28:53 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id A304C8FC08; Fri, 24 Sep 2010 13:28:53 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id 55A0B46B35; Fri, 24 Sep 2010 09:28:53 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id 57A0E8A04E; Fri, 24 Sep 2010 09:28:52 -0400 (EDT) From: John Baldwin To: freebsd-threads@freebsd.org Date: Fri, 24 Sep 2010 09:26:12 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009232348.45201.jkim@FreeBSD.org> In-Reply-To: <201009232348.45201.jkim@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009240926.12958.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Fri, 24 Sep 2010 09:28:52 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Daniel Eischen , Jung-uk Kim Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 13:28:53 -0000 On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: > On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > > You shouldn't have to call pthread_mutex_init() on a mutex > > initialized with PTHREAD_MUTEX_INITIALIZER. Our implementation > > should auto initialize the mutex when it is first used; if it > > doesn't, I think that is a bug. > > Ah, I see. I verified that libthr does it correctly. However, that's > a hack and it is far from real static allocation although it should > work pretty well in reality, IMHO. More over, it will have a > side-effect, i.e., any destroyed mutex may be resurrected if it is > used again. POSIX seems to say it should return EINVAL when it > happens. :-( I think the fix there is that we should put a different value ((void *)1 for example) into "destroyed" mutex objects than 0 so that destroyed mutexes can be differentiated from statically initialized mutexes. This would also allow us to properly return EBUSY, etc. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 15:38:03 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 8BEC5106564A; Fri, 24 Sep 2010 15:38:03 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: John Baldwin Date: Fri, 24 Sep 2010 11:37:53 -0400 User-Agent: KMail/1.6.2 References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009232348.45201.jkim@FreeBSD.org> <201009240926.12958.jhb@freebsd.org> In-Reply-To: <201009240926.12958.jhb@freebsd.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009241137.56764.jkim@FreeBSD.org> Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 15:38:03 -0000 On Friday 24 September 2010 09:26 am, John Baldwin wrote: > On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: > > On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > > > You shouldn't have to call pthread_mutex_init() on a mutex > > > initialized with PTHREAD_MUTEX_INITIALIZER. Our implementation > > > should auto initialize the mutex when it is first used; if it > > > doesn't, I think that is a bug. > > > > Ah, I see. I verified that libthr does it correctly. However, > > that's a hack and it is far from real static allocation although > > it should work pretty well in reality, IMHO. More over, it will > > have a side-effect, i.e., any destroyed mutex may be resurrected > > if it is used again. POSIX seems to say it should return EINVAL > > when it happens. :-( > > I think the fix there is that we should put a different value > ((void *)1 for example) into "destroyed" mutex objects than 0 so > that destroyed mutexes can be differentiated from statically > initialized mutexes. This would also allow us to properly return > EBUSY, etc. It would be nice if we had "uninitialized" as (void *)0 and "static initializer" as (void *)1. IMHO, it looks more natural, i.e., "uninitialized" or "destroyed" one gets zero, and "dynamically initialized" or "statically initialized" one gets non-zero. Can we break the ABI for 9, maybe? ;-) Jung-uk Kim From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 21:11:31 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 757FA106566B; Fri, 24 Sep 2010 21:11:22 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: John Baldwin Date: Fri, 24 Sep 2010 17:11:02 -0400 User-Agent: KMail/1.6.2 References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009240926.12958.jhb@freebsd.org> <201009241137.56764.jkim@FreeBSD.org> In-Reply-To: <201009241137.56764.jkim@FreeBSD.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: Multipart/Mixed; boundary="Boundary-00=_0PRnME9TC8tgtzT" Message-Id: <201009241711.16129.jkim@FreeBSD.org> Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 21:11:31 -0000 --Boundary-00=_0PRnME9TC8tgtzT Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Friday 24 September 2010 11:37 am, Jung-uk Kim wrote: > On Friday 24 September 2010 09:26 am, John Baldwin wrote: > > On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: > > > On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > > > > You shouldn't have to call pthread_mutex_init() on a mutex > > > > initialized with PTHREAD_MUTEX_INITIALIZER. Our > > > > implementation should auto initialize the mutex when it is > > > > first used; if it doesn't, I think that is a bug. > > > > > > Ah, I see. I verified that libthr does it correctly. However, > > > that's a hack and it is far from real static allocation > > > although it should work pretty well in reality, IMHO. More > > > over, it will have a side-effect, i.e., any destroyed mutex may > > > be resurrected if it is used again. POSIX seems to say it > > > should return EINVAL when it happens. :-( > > > > I think the fix there is that we should put a different value > > ((void *)1 for example) into "destroyed" mutex objects than 0 so > > that destroyed mutexes can be differentiated from statically > > initialized mutexes. This would also allow us to properly return > > EBUSY, etc. > > It would be nice if we had "uninitialized" as (void *)0 and "static > initializer" as (void *)1. IMHO, it looks more natural, i.e., > "uninitialized" or "destroyed" one gets zero, and "dynamically > initialized" or "statically initialized" one gets non-zero. Can we > break the ABI for 9, maybe? ;-) -------------------------------- WARNING!!! WARNING!!! WARNING!!! -------------------------------- This patch is a proof of concept! This patch massively breaks pthread ABI! This patch may crash and burn your system! I couldn't resist trying. ;-) Jung-uk Kim --Boundary-00=_0PRnME9TC8tgtzT Content-Type: text/plain; charset="iso-8859-1"; name="pthread.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pthread.diff" --- include/pthread.h 2009-03-14 16:10:14.000000000 -0400 +++ include/pthread.h 2010-09-24 16:34:45.000000000 -0400 @@ -97,10 +97,10 @@ /* * Static initialization values. */ -#define PTHREAD_MUTEX_INITIALIZER NULL -#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP NULL -#define PTHREAD_COND_INITIALIZER NULL -#define PTHREAD_RWLOCK_INITIALIZER NULL +#define PTHREAD_MUTEX_INITIALIZER ((void *)1) +#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP ((void *)1) +#define PTHREAD_COND_INITIALIZER ((void *)1) +#define PTHREAD_RWLOCK_INITIALIZER ((void *)1) /* * Default attribute arguments (draft 4, deprecated). --- lib/libthr/thread/thr_cond.c 2010-09-01 12:13:31.000000000 -0400 +++ lib/libthr/thread/thr_cond.c 2010-09-24 15:36:56.000000000 -0400 @@ -94,7 +94,7 @@ init_static(struct pthread *thread, pthr THR_LOCK_ACQUIRE(thread, &_cond_static_lock); - if (*cond == NULL) + if (*cond == STATIC_INITIALIZER) ret = cond_init(cond, NULL); else ret = 0; @@ -121,6 +121,8 @@ _pthread_cond_destroy(pthread_cond_t *co if (*cond == NULL) rval = EINVAL; + else if (*cond == STATIC_INITIALIZER) + *cond = NULL; else { cv = *cond; THR_UMUTEX_LOCK(curthread, &cv->c_lock); @@ -184,7 +186,7 @@ cond_wait_common(pthread_cond_t *cond, p * If the condition variable is statically initialized, * perform the dynamic initialization: */ - if (__predict_false(*cond == NULL && + if (__predict_false(*cond == STATIC_INITIALIZER && (ret = init_static(curthread, cond)) != 0)) return (ret); @@ -273,7 +275,7 @@ cond_signal_common(pthread_cond_t *cond, * If the condition variable is statically initialized, perform dynamic * initialization. */ - if (__predict_false(*cond == NULL && + if (__predict_false(*cond == STATIC_INITIALIZER && (ret = init_static(curthread, cond)) != 0)) return (ret); --- lib/libthr/thread/thr_mutex.c 2010-09-01 12:13:32.000000000 -0400 +++ lib/libthr/thread/thr_mutex.c 2010-09-24 16:32:07.000000000 -0400 @@ -184,7 +184,7 @@ init_static(struct pthread *thread, pthr THR_LOCK_ACQUIRE(thread, &_mutex_static_lock); - if (*mutex == NULL) + if (*mutex == STATIC_INITIALIZER) ret = mutex_init(mutex, NULL, calloc); else ret = 0; @@ -263,6 +263,8 @@ _pthread_mutex_destroy(pthread_mutex_t * if (__predict_false(*mutex == NULL)) ret = EINVAL; + else if (*mutex == STATIC_INITIALIZER) + *mutex = NULL; else { id = TID(curthread); @@ -346,7 +348,7 @@ __pthread_mutex_trylock(pthread_mutex_t * If the mutex is statically initialized, perform the dynamic * initialization: */ - if (__predict_false(*mutex == NULL)) { + if (__predict_false(*mutex == STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -454,7 +456,7 @@ __pthread_mutex_lock(pthread_mutex_t *mu * If the mutex is statically initialized, perform the dynamic * initialization: */ - if (__predict_false((m = *mutex) == NULL)) { + if (__predict_false((m = *mutex) == STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -479,7 +481,7 @@ __pthread_mutex_timedlock(pthread_mutex_ * If the mutex is statically initialized, perform the dynamic * initialization: */ - if (__predict_false((m = *mutex) == NULL)) { + if (__predict_false((m = *mutex) == STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -614,6 +616,9 @@ mutex_unlock_common(pthread_mutex_t *mut if (__predict_false((m = *mutex) == NULL)) return (EINVAL); + if (*mutex == STATIC_INITIALIZER) + return (EPERM); + /* * Check if the running thread is not the owner of the mutex. */ @@ -749,7 +754,7 @@ __pthread_mutex_setspinloops_np(pthread_ struct pthread *curthread = _get_curthread(); int ret; - if (__predict_false(*mutex == NULL)) { + if (__predict_false(*mutex == STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -773,7 +778,7 @@ __pthread_mutex_setyieldloops_np(pthread struct pthread *curthread = _get_curthread(); int ret; - if (__predict_false(*mutex == NULL)) { + if (__predict_false(*mutex == STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -788,7 +793,7 @@ _pthread_mutex_isowned_np(pthread_mutex_ struct pthread *curthread = _get_curthread(); int ret; - if (__predict_false(*mutex == NULL)) { + if (__predict_false(*mutex == STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); --- lib/libthr/thread/thr_private.h 2010-09-24 12:01:02.000000000 -0400 +++ lib/libthr/thread/thr_private.h 2010-09-24 14:43:35.000000000 -0400 @@ -125,6 +125,8 @@ TAILQ_HEAD(mutex_queue, pthread_mutex); } \ } while (0) +#define STATIC_INITIALIZER ((void *)1) + struct pthread_mutex { /* * Lock for accesses to this structure. --- lib/libthr/thread/thr_rwlock.c 2009-07-06 05:31:04.000000000 -0400 +++ lib/libthr/thread/thr_rwlock.c 2010-09-24 16:45:37.000000000 -0400 @@ -64,10 +64,12 @@ rwlock_init(pthread_rwlock_t *rwlock, co int _pthread_rwlock_destroy (pthread_rwlock_t *rwlock) { - int ret; + int ret = 0; if (rwlock == NULL) ret = EINVAL; + else if (*rwlock == STATIC_INITIALIZER) + *rwlock = NULL; else { pthread_rwlock_t prwlock; @@ -75,7 +77,6 @@ _pthread_rwlock_destroy (pthread_rwlock_ *rwlock = NULL; free(prwlock); - ret = 0; } return (ret); } @@ -87,7 +88,7 @@ init_static(struct pthread *thread, pthr THR_LOCK_ACQUIRE(thread, &_rwlock_static_lock); - if (*rwlock == NULL) + if (*rwlock == STATIC_INITIALIZER) ret = rwlock_init(rwlock, NULL); else ret = 0; @@ -119,7 +120,7 @@ rwlock_rdlock_common(pthread_rwlock_t *r prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -212,7 +213,7 @@ _pthread_rwlock_tryrdlock (pthread_rwloc prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -256,7 +257,7 @@ _pthread_rwlock_trywrlock (pthread_rwloc prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -283,7 +284,7 @@ rwlock_wrlock_common (pthread_rwlock_t * prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -364,6 +365,9 @@ _pthread_rwlock_unlock (pthread_rwlock_t if (__predict_false(prwlock == NULL)) return (EINVAL); + if (prwlock == STATIC_INITIALIZER) + return (EPERM); + state = prwlock->lock.rw_state; if (state & URWLOCK_WRITE_OWNER) { if (__predict_false(prwlock->owner != curthread)) --Boundary-00=_0PRnME9TC8tgtzT-- From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 21:27:45 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 16B56106567A; Fri, 24 Sep 2010 21:27:45 +0000 (UTC) (envelope-from kabaev@gmail.com) Received: from mail-qy0-f175.google.com (mail-qy0-f175.google.com [209.85.216.175]) by mx1.freebsd.org (Postfix) with ESMTP id 784BE8FC13; Fri, 24 Sep 2010 21:27:44 +0000 (UTC) Received: by mail-qy0-f175.google.com with SMTP id 31so1981516qyk.13 for ; Fri, 24 Sep 2010 14:27:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:date:from:to:cc:subject :message-id:in-reply-to:references:x-mailer:mime-version :content-type; bh=Vkvme/nijwmindVObxuh5Omt1bYIkGPinZmzN9faTaM=; b=oL1p3uA8cBy2C1kYksJc9rQvvIDd0MqYPejGIm0JlT0S6wK7yw4aicQY2hYBPJCtgk C9NjVciZ3MXyhkLNE7bs8Se4mkl61LWaYvftXmtPy0kmt9tWRGleEl1xILa70VY4zL1q W3r9P6SlO7JMVtIN+S6akqzhIy0dnhGdQvmvU= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=date:from:to:cc:subject:message-id:in-reply-to:references:x-mailer :mime-version:content-type; b=otSkMnrrzPtPDbVFxbimpu6DAjZv5BtgZw6xXrL9ZcjibrEkWL+4lorSkemzIWs6OP GJwyMRLQQl+4bm7Smyc9pQ7RqZgQjGL9abTUKia7btIWShD644nR5PGXwvkGzRAY4zQm sdimdA3wQlojSW9naw7fr85ld7tdrg9rHO/oI= Received: by 10.229.188.149 with SMTP id da21mr3022379qcb.84.1285363663971; Fri, 24 Sep 2010 14:27:43 -0700 (PDT) Received: from kan.dnsalias.net (c-24-63-226-98.hsd1.ma.comcast.net [24.63.226.98]) by mx.google.com with ESMTPS id t24sm2819424qcs.23.2010.09.24.14.27.42 (version=SSLv3 cipher=RC4-MD5); Fri, 24 Sep 2010 14:27:42 -0700 (PDT) Date: Fri, 24 Sep 2010 17:27:35 -0400 From: Alexander Kabaev To: Jung-uk Kim Message-ID: <20100924172735.6b3910d1@kan.dnsalias.net> In-Reply-To: <201009241711.16129.jkim@FreeBSD.org> References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009240926.12958.jhb@freebsd.org> <201009241137.56764.jkim@FreeBSD.org> <201009241711.16129.jkim@FreeBSD.org> X-Mailer: Claws Mail 3.7.6 (GTK+ 2.20.1; amd64-portbld-freebsd9.0) Mime-Version: 1.0 Content-Type: multipart/signed; micalg=PGP-SHA1; boundary="Sig_/8XS8I3ndhdfPngf5dJYO8=C"; protocol="application/pgp-signature" Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 21:27:45 -0000 --Sig_/8XS8I3ndhdfPngf5dJYO8=C Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: quoted-printable If we were to break PTHREAD ABI, we should go all the way and make necessary changed to make process-shared locks possible. All IMHO. --=20 Alexander Kabaev --Sig_/8XS8I3ndhdfPngf5dJYO8=C Content-Type: application/pgp-signature; name=signature.asc Content-Disposition: attachment; filename=signature.asc -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.16 (FreeBSD) iD8DBQFMnRfNQ6z1jMm+XZYRAposAJ4x5iavuPQjpxPfvJlYk5/1SBx+/gCZAQ6J d8n3z2C1u6LV6T9iO/KICv4= =2flU -----END PGP SIGNATURE----- --Sig_/8XS8I3ndhdfPngf5dJYO8=C-- From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 21:29:22 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 6D19210656AA; Fri, 24 Sep 2010 21:29:22 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 3CC188FC0A; Fri, 24 Sep 2010 21:29:22 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id E289546B2C; Fri, 24 Sep 2010 17:29:21 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id DB95D8A04E; Fri, 24 Sep 2010 17:29:20 -0400 (EDT) From: John Baldwin To: "Jung-uk Kim" Date: Fri, 24 Sep 2010 17:24:10 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009240926.12958.jhb@freebsd.org> <201009241137.56764.jkim@FreeBSD.org> In-Reply-To: <201009241137.56764.jkim@FreeBSD.org> MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009241724.10223.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Fri, 24 Sep 2010 17:29:21 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 21:29:22 -0000 On Friday, September 24, 2010 11:37:53 am Jung-uk Kim wrote: > On Friday 24 September 2010 09:26 am, John Baldwin wrote: > > On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: > > > On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > > > > You shouldn't have to call pthread_mutex_init() on a mutex > > > > initialized with PTHREAD_MUTEX_INITIALIZER. Our implementation > > > > should auto initialize the mutex when it is first used; if it > > > > doesn't, I think that is a bug. > > > > > > Ah, I see. I verified that libthr does it correctly. However, > > > that's a hack and it is far from real static allocation although > > > it should work pretty well in reality, IMHO. More over, it will > > > have a side-effect, i.e., any destroyed mutex may be resurrected > > > if it is used again. POSIX seems to say it should return EINVAL > > > when it happens. :-( > > > > I think the fix there is that we should put a different value > > ((void *)1 for example) into "destroyed" mutex objects than 0 so > > that destroyed mutexes can be differentiated from statically > > initialized mutexes. This would also allow us to properly return > > EBUSY, etc. > > It would be nice if we had "uninitialized" as (void *)0 and "static > initializer" as (void *)1. IMHO, it looks more natural, i.e., > "uninitialized" or "destroyed" one gets zero, and "dynamically > initialized" or "statically initialized" one gets non-zero. Can we > break the ABI for 9, maybe? ;-) I actually find the (void *)1 more natural for a destroyed state FWIW. One thing I would advocate is that regardless of the values chosen, use constants for both the INITIALIZER and DESTROYED values. That would make the code more obvious. In general I think your patch in your followup is correct, but having explicitly DESTROYED constants that you check against instead of NULL would improve the code. -- John Baldwin From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 23:37:12 2010 Return-Path: Delivered-To: freebsd-threads@FreeBSD.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 21B7A106566B; Fri, 24 Sep 2010 23:37:12 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: Alexander Kabaev Date: Fri, 24 Sep 2010 19:37:03 -0400 User-Agent: KMail/1.6.2 References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009241711.16129.jkim@FreeBSD.org> <20100924172735.6b3910d1@kan.dnsalias.net> In-Reply-To: <20100924172735.6b3910d1@kan.dnsalias.net> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009241937.05173.jkim@FreeBSD.org> Cc: Daniel Eischen , John Baldwin , freebsd-threads@FreeBSD.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 23:37:12 -0000 On Friday 24 September 2010 05:27 pm, Alexander Kabaev wrote: > If we were to break PTHREAD ABI, we should go all the way and > make necessary changed to make process-shared locks possible. > All IMHO. I agree. Jung-uk Kim From owner-freebsd-threads@FreeBSD.ORG Fri Sep 24 23:43:18 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 833C3106566C; Fri, 24 Sep 2010 23:43:17 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: John Baldwin Date: Fri, 24 Sep 2010 19:43:09 -0400 User-Agent: KMail/1.6.2 References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009241137.56764.jkim@FreeBSD.org> <201009241724.10223.jhb@freebsd.org> In-Reply-To: <201009241724.10223.jhb@freebsd.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: Multipart/Mixed; boundary="Boundary-00=_OeTnMm3Rs0n8fas" Message-Id: <201009241943.10401.jkim@FreeBSD.org> Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Fri, 24 Sep 2010 23:43:18 -0000 --Boundary-00=_OeTnMm3Rs0n8fas Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Content-Disposition: inline On Friday 24 September 2010 05:24 pm, John Baldwin wrote: > On Friday, September 24, 2010 11:37:53 am Jung-uk Kim wrote: > > On Friday 24 September 2010 09:26 am, John Baldwin wrote: > > > On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: > > > > On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > > > > > You shouldn't have to call pthread_mutex_init() on a mutex > > > > > initialized with PTHREAD_MUTEX_INITIALIZER. Our > > > > > implementation should auto initialize the mutex when it is > > > > > first used; if it doesn't, I think that is a bug. > > > > > > > > Ah, I see. I verified that libthr does it correctly. > > > > However, that's a hack and it is far from real static > > > > allocation although it should work pretty well in reality, > > > > IMHO. More over, it will have a side-effect, i.e., any > > > > destroyed mutex may be resurrected if it is used again. > > > > POSIX seems to say it should return EINVAL when it happens. > > > > :-( > > > > > > I think the fix there is that we should put a different value > > > ((void *)1 for example) into "destroyed" mutex objects than 0 > > > so that destroyed mutexes can be differentiated from statically > > > initialized mutexes. This would also allow us to properly > > > return EBUSY, etc. > > > > It would be nice if we had "uninitialized" as (void *)0 and > > "static initializer" as (void *)1. IMHO, it looks more natural, > > i.e., "uninitialized" or "destroyed" one gets zero, and > > "dynamically initialized" or "statically initialized" one gets > > non-zero. Can we break the ABI for 9, maybe? ;-) > > I actually find the (void *)1 more natural for a destroyed state > FWIW. One thing I would advocate is that regardless of the values > chosen, use constants for both the INITIALIZER and DESTROYED > values. That would make the code more obvious. In general I think > your patch in your followup is correct, but having explicitly > DESTROYED constants that you check against instead of NULL would > improve the code. Here goes more complicated patch. Not really tested, though. Jung-uk Kim --Boundary-00=_OeTnMm3Rs0n8fas Content-Type: text/plain; charset="iso-8859-1"; name="pthread2.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="pthread2.diff" --- include/pthread.h 2010-09-24 16:49:50.000000000 -0400 +++ include/pthread.h 2010-09-24 17:42:00.000000000 -0400 @@ -97,10 +97,10 @@ /* * Static initialization values. */ -#define PTHREAD_MUTEX_INITIALIZER NULL -#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP NULL -#define PTHREAD_COND_INITIALIZER NULL -#define PTHREAD_RWLOCK_INITIALIZER NULL +#define PTHREAD_MUTEX_INITIALIZER ((void *)1) +#define PTHREAD_ADAPTIVE_MUTEX_INITIALIZER_NP ((void *)1) +#define PTHREAD_COND_INITIALIZER ((void *)1) +#define PTHREAD_RWLOCK_INITIALIZER ((void *)1) /* * Default attribute arguments (draft 4, deprecated). --- lib/libthr/thread/thr_cond.c 2010-09-24 16:49:50.000000000 -0400 +++ lib/libthr/thread/thr_cond.c 2010-09-24 19:32:34.000000000 -0400 @@ -64,27 +64,28 @@ static int cond_init(pthread_cond_t *cond, const pthread_condattr_t *cond_attr) { pthread_cond_t pcond; - int rval = 0; - if ((pcond = (pthread_cond_t) - calloc(1, sizeof(struct pthread_cond))) == NULL) { - rval = ENOMEM; + if (__predict_false(cond == NULL)) + return (EINVAL); + + pcond = (pthread_cond_t)calloc(1, sizeof(struct pthread_cond)); + if (pcond == NULL) + return (ENOMEM); + + /* + * Initialise the condition variable structure: + */ + if (cond_attr == NULL || *cond_attr == NULL) { + pcond->c_pshared = 0; + pcond->c_clockid = CLOCK_REALTIME; } else { - /* - * Initialise the condition variable structure: - */ - if (cond_attr == NULL || *cond_attr == NULL) { - pcond->c_pshared = 0; - pcond->c_clockid = CLOCK_REALTIME; - } else { - pcond->c_pshared = (*cond_attr)->c_pshared; - pcond->c_clockid = (*cond_attr)->c_clockid; - } - _thr_umutex_init(&pcond->c_lock); - *cond = pcond; + pcond->c_pshared = (*cond_attr)->c_pshared; + pcond->c_clockid = (*cond_attr)->c_clockid; } - /* Return the completion status: */ - return (rval); + _thr_umutex_init(&pcond->c_lock); + *cond = pcond; + + return (0); } static int @@ -94,7 +95,7 @@ init_static(struct pthread *thread, pthr THR_LOCK_ACQUIRE(thread, &_cond_static_lock); - if (*cond == NULL) + if (*cond == THR_STATIC_INITIALIZER) ret = cond_init(cond, NULL); else ret = 0; @@ -108,7 +109,6 @@ int _pthread_cond_init(pthread_cond_t *cond, const pthread_condattr_t *cond_attr) { - *cond = NULL; return (cond_init(cond, cond_attr)); } @@ -117,10 +117,12 @@ _pthread_cond_destroy(pthread_cond_t *co { struct pthread *curthread = _get_curthread(); struct pthread_cond *cv; - int rval = 0; - if (*cond == NULL) - rval = EINVAL; + if (__predict_false(cond == NULL || *cond == THR_UNINITIALIZED)) + return (EINVAL); + + if (*cond == THR_STATIC_INITIALIZER) + *cond = THR_UNINITIALIZED; else { cv = *cond; THR_UMUTEX_LOCK(curthread, &cv->c_lock); @@ -128,7 +130,7 @@ _pthread_cond_destroy(pthread_cond_t *co * NULL the caller's pointer now that the condition * variable has been destroyed: */ - *cond = NULL; + *cond = THR_UNINITIALIZED; THR_UMUTEX_UNLOCK(curthread, &cv->c_lock); /* @@ -137,8 +139,8 @@ _pthread_cond_destroy(pthread_cond_t *co */ free(cv); } - /* Return the completion status: */ - return (rval); + + return (0); } struct cond_cancel_info @@ -184,7 +186,7 @@ cond_wait_common(pthread_cond_t *cond, p * If the condition variable is statically initialized, * perform the dynamic initialization: */ - if (__predict_false(*cond == NULL && + if (__predict_false(*cond == THR_STATIC_INITIALIZER && (ret = init_static(curthread, cond)) != 0)) return (ret); @@ -273,7 +275,7 @@ cond_signal_common(pthread_cond_t *cond, * If the condition variable is statically initialized, perform dynamic * initialization. */ - if (__predict_false(*cond == NULL && + if (__predict_false(*cond == THR_STATIC_INITIALIZER && (ret = init_static(curthread, cond)) != 0)) return (ret); --- /usr/src/lib/libthr/thread/thr_mutex.c 2010-09-24 16:49:50.000000000 -0400 +++ lib/libthr/thread/thr_mutex.c 2010-09-24 19:25:30.000000000 -0400 @@ -130,9 +130,12 @@ mutex_init(pthread_mutex_t *mutex, const struct pthread_mutex_attr *attr; struct pthread_mutex *pmutex; - if (mutex_attr == NULL) { + if (__predict_false(mutex == NULL)) + return (EINVAL); + + if (mutex_attr == NULL) attr = &_pthread_mutexattr_default; - } else { + else { attr = *mutex_attr; if (attr->m_type < PTHREAD_MUTEX_ERRORCHECK || attr->m_type >= PTHREAD_MUTEX_TYPE_MAX) @@ -141,8 +144,8 @@ mutex_init(pthread_mutex_t *mutex, attr->m_protocol > PTHREAD_PRIO_PROTECT) return (EINVAL); } - if ((pmutex = (pthread_mutex_t) - calloc_cb(1, sizeof(struct pthread_mutex))) == NULL) + pmutex = (pthread_mutex_t)calloc_cb(1, sizeof(struct pthread_mutex)); + if (pmutex == NULL) return (ENOMEM); pmutex->m_type = attr->m_type; @@ -184,7 +187,7 @@ init_static(struct pthread *thread, pthr THR_LOCK_ACQUIRE(thread, &_mutex_static_lock); - if (*mutex == NULL) + if (*mutex == THR_STATIC_INITIALIZER) ret = mutex_init(mutex, NULL, calloc); else ret = 0; @@ -259,48 +262,53 @@ _pthread_mutex_destroy(pthread_mutex_t * struct pthread *curthread = _get_curthread(); pthread_mutex_t m; uint32_t id; - int ret = 0; + int ret; - if (__predict_false(*mutex == NULL)) - ret = EINVAL; - else { - id = TID(curthread); + if (__predict_false(mutex == NULL || *mutex == THR_UNINITIALIZED)) + return (EINVAL); + if (*mutex == THR_STATIC_INITIALIZER) { + *mutex = THR_UNINITIALIZED; + return (0); + } + + id = TID(curthread); + + /* + * Try to lock the mutex structure, we only need to try once, + * if failed, the mutex is in used. + */ + ret = _thr_umutex_trylock(&(*mutex)->m_lock, id); + if (ret) + return (ret); + + /* + * Check mutex other fields to see if this mutex is in use. + * Mostly for prority mutex types, or there are condition + * variables referencing it. + */ + m = *mutex; + if (m->m_owner != NULL || m->m_refcount != 0) { + if (m->m_lock.m_flags & UMUTEX_PRIO_PROTECT) + set_inherited_priority(curthread, m); + _thr_umutex_unlock(&m->m_lock, id); + return (EBUSY); + } else { /* - * Try to lock the mutex structure, we only need to - * try once, if failed, the mutex is in used. - */ - ret = _thr_umutex_trylock(&(*mutex)->m_lock, id); - if (ret) - return (ret); - m = *mutex; - /* - * Check mutex other fields to see if this mutex is - * in use. Mostly for prority mutex types, or there - * are condition variables referencing it. + * Save a pointer to the mutex so it can be free'd + * and set the caller's pointer to NULL. */ - if (m->m_owner != NULL || m->m_refcount != 0) { - if (m->m_lock.m_flags & UMUTEX_PRIO_PROTECT) - set_inherited_priority(curthread, m); - _thr_umutex_unlock(&m->m_lock, id); - ret = EBUSY; - } else { - /* - * Save a pointer to the mutex so it can be free'd - * and set the caller's pointer to NULL. - */ - *mutex = NULL; + *mutex = THR_UNINITIALIZED; - if (m->m_lock.m_flags & UMUTEX_PRIO_PROTECT) - set_inherited_priority(curthread, m); - _thr_umutex_unlock(&m->m_lock, id); + if (m->m_lock.m_flags & UMUTEX_PRIO_PROTECT) + set_inherited_priority(curthread, m); + _thr_umutex_unlock(&m->m_lock, id); - MUTEX_ASSERT_NOT_OWNED(m); - free(m); - } + MUTEX_ASSERT_NOT_OWNED(m); + free(m); } - return (ret); + return (0); } #define ENQUEUE_MUTEX(curthread, m) \ @@ -342,11 +350,14 @@ __pthread_mutex_trylock(pthread_mutex_t struct pthread *curthread = _get_curthread(); int ret; + if (__predict_false(mutex == NULL)) + return (EINVAL); + /* * If the mutex is statically initialized, perform the dynamic * initialization: */ - if (__predict_false(*mutex == NULL)) { + if (__predict_false(*mutex == THR_STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -446,6 +457,9 @@ __pthread_mutex_lock(pthread_mutex_t *mu struct pthread_mutex *m; int ret; + if (__predict_false(mutex == NULL)) + return (EINVAL); + _thr_check_init(); curthread = _get_curthread(); @@ -454,7 +468,7 @@ __pthread_mutex_lock(pthread_mutex_t *mu * If the mutex is statically initialized, perform the dynamic * initialization: */ - if (__predict_false((m = *mutex) == NULL)) { + if (__predict_false((m = *mutex) == THR_STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -471,6 +485,9 @@ __pthread_mutex_timedlock(pthread_mutex_ struct pthread_mutex *m; int ret; + if (__predict_false(mutex == NULL)) + return (EINVAL); + _thr_check_init(); curthread = _get_curthread(); @@ -479,7 +496,7 @@ __pthread_mutex_timedlock(pthread_mutex_ * If the mutex is statically initialized, perform the dynamic * initialization: */ - if (__predict_false((m = *mutex) == NULL)) { + if (__predict_false((m = *mutex) == THR_STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -611,9 +628,12 @@ mutex_unlock_common(pthread_mutex_t *mut struct pthread_mutex *m; uint32_t id; - if (__predict_false((m = *mutex) == NULL)) + if (__predict_false(mutex == NULL || (m = *mutex) == THR_UNINITIALIZED)) return (EINVAL); + if (*mutex == THR_STATIC_INITIALIZER) + return (EPERM); + /* * Check if the running thread is not the owner of the mutex. */ @@ -649,7 +669,7 @@ _mutex_cv_unlock(pthread_mutex_t *mutex, struct pthread *curthread = _get_curthread(); struct pthread_mutex *m; - if (__predict_false((m = *mutex) == NULL)) + if (__predict_false((m = *mutex) == THR_UNINITIALIZED)) return (EINVAL); /* @@ -685,18 +705,14 @@ int _pthread_mutex_getprioceiling(pthread_mutex_t *mutex, int *prioceiling) { - int ret; - if (*mutex == NULL) - ret = EINVAL; - else if (((*mutex)->m_lock.m_flags & UMUTEX_PRIO_PROTECT) == 0) - ret = EINVAL; - else { - *prioceiling = (*mutex)->m_lock.m_ceilings[0]; - ret = 0; - } + if (mutex == NULL || *mutex == THR_UNINITIALIZED || + ((*mutex)->m_lock.m_flags & UMUTEX_PRIO_PROTECT) == 0) + return (EINVAL); + + *prioceiling = (*mutex)->m_lock.m_ceilings[0]; - return(ret); + return (0); } int @@ -707,10 +723,11 @@ _pthread_mutex_setprioceiling(pthread_mu struct pthread_mutex *m, *m1, *m2; int ret; - m = *mutex; - if (m == NULL || (m->m_lock.m_flags & UMUTEX_PRIO_PROTECT) == 0) + if (mutex == NULL || *mutex == THR_UNINITIALIZED || + ((*mutex)->m_lock.m_flags & UMUTEX_PRIO_PROTECT) == 0) return (EINVAL); + m = *mutex; ret = __thr_umutex_set_ceiling(&m->m_lock, ceiling, old_ceiling); if (ret != 0) return (ret); @@ -737,7 +754,8 @@ _pthread_mutex_setprioceiling(pthread_mu int _pthread_mutex_getspinloops_np(pthread_mutex_t *mutex, int *count) { - if (*mutex == NULL) + + if (mutex == NULL || *mutex == THR_UNINITIALIZED) return (EINVAL); *count = (*mutex)->m_spinloops; return (0); @@ -749,7 +767,7 @@ __pthread_mutex_setspinloops_np(pthread_ struct pthread *curthread = _get_curthread(); int ret; - if (__predict_false(*mutex == NULL)) { + if (__predict_false(*mutex == THR_STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -761,7 +779,8 @@ __pthread_mutex_setspinloops_np(pthread_ int _pthread_mutex_getyieldloops_np(pthread_mutex_t *mutex, int *count) { - if (*mutex == NULL) + + if (mutex == NULL || *mutex == THR_UNINITIALIZED) return (EINVAL); *count = (*mutex)->m_yieldloops; return (0); @@ -773,7 +792,9 @@ __pthread_mutex_setyieldloops_np(pthread struct pthread *curthread = _get_curthread(); int ret; - if (__predict_false(*mutex == NULL)) { + if (__predict_false(mutex == NULL)) + return (EINVAL); + if (__predict_false(*mutex == THR_STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); @@ -788,7 +809,9 @@ _pthread_mutex_isowned_np(pthread_mutex_ struct pthread *curthread = _get_curthread(); int ret; - if (__predict_false(*mutex == NULL)) { + if (__predict_false(mutex == NULL)) + return (EINVAL); + if (__predict_false(*mutex == THR_STATIC_INITIALIZER)) { ret = init_static(curthread, mutex); if (__predict_false(ret)) return (ret); --- lib/libthr/thread/thr_private.h 2010-09-24 16:49:50.000000000 -0400 +++ lib/libthr/thread/thr_private.h 2010-09-24 17:45:12.000000000 -0400 @@ -125,6 +125,9 @@ TAILQ_HEAD(mutex_queue, pthread_mutex); } \ } while (0) +#define THR_UNINITIALIZED ((void *)0) +#define THR_STATIC_INITIALIZER ((void *)1) + struct pthread_mutex { /* * Lock for accesses to this structure. --- lib/libthr/thread/thr_rwlock.c 2010-09-24 16:49:50.000000000 -0400 +++ lib/libthr/thread/thr_rwlock.c 2010-09-24 19:26:20.000000000 -0400 @@ -54,6 +54,9 @@ rwlock_init(pthread_rwlock_t *rwlock, co { pthread_rwlock_t prwlock; + if (__predict_false(rwlock == NULL)) + return (EINVAL); + prwlock = (pthread_rwlock_t)calloc(1, sizeof(struct pthread_rwlock)); if (prwlock == NULL) return (ENOMEM); @@ -64,20 +67,14 @@ rwlock_init(pthread_rwlock_t *rwlock, co int _pthread_rwlock_destroy (pthread_rwlock_t *rwlock) { - int ret; - if (rwlock == NULL) - ret = EINVAL; - else { - pthread_rwlock_t prwlock; - - prwlock = *rwlock; - *rwlock = NULL; + if (__predict_false(rwlock == NULL || *rwlock == THR_UNINITIALIZED)) + return (EINVAL); - free(prwlock); - ret = 0; - } - return (ret); + if (*rwlock != THR_STATIC_INITIALIZER) + free(*rwlock); + *rwlock = THR_UNINITIALIZED; + return (0); } static int @@ -87,7 +84,7 @@ init_static(struct pthread *thread, pthr THR_LOCK_ACQUIRE(thread, &_rwlock_static_lock); - if (*rwlock == NULL) + if (*rwlock == THR_STATIC_INITIALIZER) ret = rwlock_init(rwlock, NULL); else ret = 0; @@ -100,7 +97,7 @@ init_static(struct pthread *thread, pthr int _pthread_rwlock_init (pthread_rwlock_t *rwlock, const pthread_rwlockattr_t *attr) { - *rwlock = NULL; + return (rwlock_init(rwlock, attr)); } @@ -119,7 +116,7 @@ rwlock_rdlock_common(pthread_rwlock_t *r prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == THR_STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -212,7 +209,7 @@ _pthread_rwlock_tryrdlock (pthread_rwloc prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == THR_STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -256,7 +253,7 @@ _pthread_rwlock_trywrlock (pthread_rwloc prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == THR_STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -283,7 +280,7 @@ rwlock_wrlock_common (pthread_rwlock_t * prwlock = *rwlock; /* check for static initialization */ - if (__predict_false(prwlock == NULL)) { + if (__predict_false(prwlock == THR_STATIC_INITIALIZER)) { if ((ret = init_static(curthread, rwlock)) != 0) return (ret); @@ -361,9 +358,12 @@ _pthread_rwlock_unlock (pthread_rwlock_t prwlock = *rwlock; - if (__predict_false(prwlock == NULL)) + if (__predict_false(prwlock == THR_UNINITIALIZED)) return (EINVAL); + if (prwlock == THR_STATIC_INITIALIZER) + return (EPERM); + state = prwlock->lock.rw_state; if (state & URWLOCK_WRITE_OWNER) { if (__predict_false(prwlock->owner != curthread)) --Boundary-00=_OeTnMm3Rs0n8fas-- From owner-freebsd-threads@FreeBSD.ORG Sat Sep 25 01:24:42 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 546D31065673; Sat, 25 Sep 2010 01:24:42 +0000 (UTC) (envelope-from davidxu@freebsd.org) Received: from freefall.freebsd.org (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by mx1.freebsd.org (Postfix) with ESMTP id 3BEAF8FC27; Sat, 25 Sep 2010 01:24:42 +0000 (UTC) Received: from [127.0.0.1] (localhost [127.0.0.1]) by freefall.freebsd.org (8.14.4/8.14.4) with ESMTP id o8P1OdNv065712; Sat, 25 Sep 2010 01:24:40 GMT (envelope-from davidxu@freebsd.org) Message-ID: <4C9DBFD9.5000004@freebsd.org> Date: Sat, 25 Sep 2010 09:24:41 +0000 From: David Xu User-Agent: Thunderbird 2.0.0.24 (X11/20100630) MIME-Version: 1.0 To: Jung-uk Kim References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009241137.56764.jkim@FreeBSD.org> <201009241724.10223.jhb@freebsd.org> <201009241943.10401.jkim@FreeBSD.org> In-Reply-To: <201009241943.10401.jkim@FreeBSD.org> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Sat, 25 Sep 2010 01:24:42 -0000 Jung-uk Kim wrote: > On Friday 24 September 2010 05:24 pm, John Baldwin wrote: >> On Friday, September 24, 2010 11:37:53 am Jung-uk Kim wrote: >>> On Friday 24 September 2010 09:26 am, John Baldwin wrote: >>>> On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: >>>>> On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: >>>>>> You shouldn't have to call pthread_mutex_init() on a mutex >>>>>> initialized with PTHREAD_MUTEX_INITIALIZER. Our >>>>>> implementation should auto initialize the mutex when it is >>>>>> first used; if it doesn't, I think that is a bug. >>>>> Ah, I see. I verified that libthr does it correctly. >>>>> However, that's a hack and it is far from real static >>>>> allocation although it should work pretty well in reality, >>>>> IMHO. More over, it will have a side-effect, i.e., any >>>>> destroyed mutex may be resurrected if it is used again. >>>>> POSIX seems to say it should return EINVAL when it happens. >>>>> :-( >>>> I think the fix there is that we should put a different value >>>> ((void *)1 for example) into "destroyed" mutex objects than 0 >>>> so that destroyed mutexes can be differentiated from statically >>>> initialized mutexes. This would also allow us to properly >>>> return EBUSY, etc. >>> It would be nice if we had "uninitialized" as (void *)0 and >>> "static initializer" as (void *)1. IMHO, it looks more natural, >>> i.e., "uninitialized" or "destroyed" one gets zero, and >>> "dynamically initialized" or "statically initialized" one gets >>> non-zero. Can we break the ABI for 9, maybe? ;-) >> I actually find the (void *)1 more natural for a destroyed state >> FWIW. One thing I would advocate is that regardless of the values >> chosen, use constants for both the INITIALIZER and DESTROYED >> values. That would make the code more obvious. In general I think >> your patch in your followup is correct, but having explicitly >> DESTROYED constants that you check against instead of NULL would >> improve the code. > > Here goes more complicated patch. Not really tested, though. > > Jung-uk Kim > The patch touches too many lines which is unnecessary to be modified, it seems you are arbitrarily restructure the code according to you hobby. Second, breaking ABI compatible is not accepted, I think jhb@ is right here, destroyed mutex should have pointer 1 or others, non-null PTHREAD_MUTEX_INITIALIZER causes data to be put in data segment not in bss, result is large binary size, if it can be avoided, it should be avoided. Third, inserting extra branch in critical path should be caution, it is visible in performance losting in past I have compared it with competitor. I think to minimize abnormal case, I may use if (((uintptr_t)mutex) <= 1) { if (mutex == NULL) init it. else if (mutex == destroyed) return EINVAL; } This should have same overhead with current code. Regards, David Xu From owner-freebsd-threads@FreeBSD.ORG Sat Sep 25 02:53:22 2010 Return-Path: Delivered-To: freebsd-threads@freebsd.org Received: from [127.0.0.1] (freefall.freebsd.org [IPv6:2001:4f8:fff6::28]) by hub.freebsd.org (Postfix) with ESMTP id 1D3F7106564A; Sat, 25 Sep 2010 02:53:22 +0000 (UTC) (envelope-from jkim@FreeBSD.org) From: Jung-uk Kim To: David Xu Date: Fri, 24 Sep 2010 22:53:01 -0400 User-Agent: KMail/1.6.2 References: <201009232220.o8NMK3fX011639@freefall.freebsd.org> <201009241943.10401.jkim@FreeBSD.org> <4C9DBFD9.5000004@freebsd.org> In-Reply-To: <4C9DBFD9.5000004@freebsd.org> MIME-Version: 1.0 Content-Disposition: inline Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201009242253.05383.jkim@FreeBSD.org> Cc: Daniel Eischen , freebsd-threads@freebsd.org Subject: Re: threads/150889: PTHREAD_MUTEX_INITIALIZER + pthread_mutex_destroy () == EINVAL 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: Sat, 25 Sep 2010 02:53:22 -0000 On Saturday 25 September 2010 05:24 am, David Xu wrote: > Jung-uk Kim wrote: > > On Friday 24 September 2010 05:24 pm, John Baldwin wrote: > >> On Friday, September 24, 2010 11:37:53 am Jung-uk Kim wrote: > >>> On Friday 24 September 2010 09:26 am, John Baldwin wrote: > >>>> On Thursday, September 23, 2010 11:48:40 pm Jung-uk Kim wrote: > >>>>> On Thursday 23 September 2010 06:44 pm, Daniel Eischen wrote: > >>>>>> You shouldn't have to call pthread_mutex_init() on a mutex > >>>>>> initialized with PTHREAD_MUTEX_INITIALIZER. Our > >>>>>> implementation should auto initialize the mutex when it is > >>>>>> first used; if it doesn't, I think that is a bug. > >>>>> > >>>>> Ah, I see. I verified that libthr does it correctly. > >>>>> However, that's a hack and it is far from real static > >>>>> allocation although it should work pretty well in reality, > >>>>> IMHO. More over, it will have a side-effect, i.e., any > >>>>> destroyed mutex may be resurrected if it is used again. > >>>>> POSIX seems to say it should return EINVAL when it happens. > >>>>> > >>>>> :-( > >>>> > >>>> I think the fix there is that we should put a different value > >>>> ((void *)1 for example) into "destroyed" mutex objects than 0 > >>>> so that destroyed mutexes can be differentiated from > >>>> statically initialized mutexes. This would also allow us to > >>>> properly return EBUSY, etc. > >>> > >>> It would be nice if we had "uninitialized" as (void *)0 and > >>> "static initializer" as (void *)1. IMHO, it looks more > >>> natural, i.e., "uninitialized" or "destroyed" one gets zero, > >>> and "dynamically initialized" or "statically initialized" one > >>> gets non-zero. Can we break the ABI for 9, maybe? ;-) > >> > >> I actually find the (void *)1 more natural for a destroyed state > >> FWIW. One thing I would advocate is that regardless of the > >> values chosen, use constants for both the INITIALIZER and > >> DESTROYED values. That would make the code more obvious. In > >> general I think your patch in your followup is correct, but > >> having explicitly DESTROYED constants that you check against > >> instead of NULL would improve the code. > > > > Here goes more complicated patch. Not really tested, though. > > > > Jung-uk Kim > > The patch touches too many lines which is unnecessary to be > modified, it seems you are arbitrarily restructure the code > according to you hobby. Second, breaking ABI compatible is not > accepted, I think jhb@ is right here, destroyed mutex should have > pointer 1 or others, non-null PTHREAD_MUTEX_INITIALIZER causes data > to be put in data segment not in bss, result is large binary size, > if it can be avoided, it should be avoided. Third, inserting extra > branch in critical path should be caution, it is visible in > performance losting in past I have compared it with competitor. I > think to minimize abnormal case, I may use > if (((uintptr_t)mutex) <= 1) { > if (mutex == NULL) init it. > else if (mutex == destroyed) > return EINVAL; > } > This should have same overhead with current code. Don't get me wrong, I have no intention to commit the patch. In fact, I don't disagree with you. It was a proof-of-concept, friday night experiment and that's about it. Probably you missed smiley in the first patch. Sorry if I had wasted too much of your valuable time. :-/ Jung-uk Kim