On Sat, 25 Jun 2016, Konstantin Belousov wrote:

On Sat, Jun 25, 2016 at 07:14:40PM +0200, Jilles Tjoelker wrote:
On Sat, Jun 25, 2016 at 11:30:40AM +0000, Konstantin Belousov wrote:
Author: kib
Date: Sat Jun 25 11:30:40 2016
New Revision: 302194
URL: https://svnweb.freebsd.org/changeset/base/302194

Log:
  For pthread_mutex_trylock() call on owned error-check or non-portable
  adaptive mutex, return EDEADLK as required by POSIX.  The
  pthread_mutex_lock() is already compliant.

  Tested by:    Guy Yur <guy...@gmail.com>
  Sponsored by: The FreeBSD Foundation
  MFC after:    2 weeks
  Approved by:  re (gjb)

Modified:
  head/lib/libthr/thread/thr_mutex.c

Modified: head/lib/libthr/thread/thr_mutex.c
==============================================================================
--- head/lib/libthr/thread/thr_mutex.c  Sat Jun 25 10:08:04 2016        
(r302193)
+++ head/lib/libthr/thread/thr_mutex.c  Sat Jun 25 11:30:40 2016        
(r302194)
@@ -850,9 +850,12 @@ mutex_self_trylock(struct pthread_mutex

        switch (PMUTEX_TYPE(m->m_flags)) {
        case PTHREAD_MUTEX_ERRORCHECK:
-       case PTHREAD_MUTEX_NORMAL:
        case PTHREAD_MUTEX_ADAPTIVE_NP:
-               ret = EBUSY;
+               ret = EDEADLK;
+               break;
+
+       case PTHREAD_MUTEX_NORMAL:
+               ret = EBUSY;
                break;

        case PTHREAD_MUTEX_RECURSIVE:

I think POSIX (SUSv4tc1, XSH 3 pthread_mutex_lock) is clear that only
pthread_mutex_lock() can fail with [EDEADLK], not
pthread_mutex_trylock(). Instead, the error [EBUSY] listed for
pthread_mutex_trylock() applies whenever the mutex could not be acquired
because it was already locked. This includes the case where the mutex is
owned by the current thread. Note that POSIX intends to allow not
storing the owning thread's ID in non-recursive non-robust mutexes.

Failing pthread_mutex_trylock() on owned mutexes only with [EBUSY] also
matches our code before this commit and NetBSD's code, and is apparently
expected by other code.

Therefore, I think this commit should be reverted.


I already asked re for approval of the reversal and got it.  But I am still
hesitating doing the revert vs. returning EDEADLK for error-checking
mutexes.

My initial mistake was reading the statement about PTHREAD_MUTEX_ERRORCHECK
returning EDEADLK as the requirement for both functions.  It was induced
by reading the following code in samba:
https://github.com/samba-team/samba/blob/master/lib/tdb/common/mutex.c#L928
I did extracted this into stand-alone test and checked that glibc does
return EDEADLK in this case.  BTW, if somebody has Solaris machine available
to test this, I would be grateful.  Code is available at
https://www.kib.kiev.ua/kib/pshared/pthread_samba.c

I.e., plain revert would disable the only known to me consumer of the
robust mutexes. The patch which I mailed last time, returns EDEADLK for
trylock on ERRORCHECKed mutexes only. And I am tending toward glibc
compatibility there, over the literal POSIX compliance, but I want to
see the confirmation from the Klimenko' test first.

That doesn't make glibc correct.  Unless the standards change,
we should return EBUSY in this case.  It is also unexpected if an
implementation's default mutex scheme is PTHREAD_MUTEX_ERRORCHECK
and it returns EDEADLK in this case.  It's not mentioned in the
standard, and no portable application will be expecting it.

--
DE
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to