Austin Group Defect Tracker

Aardvark Mark III


Viewing Issue Simple Details Jump to Notes ] Issue History ] Print ]
ID Category Severity Type Date Submitted Last Update
0000720 [1003.1(2008)/Issue 7] Base Definitions and Headers Editorial Clarification Requested 2013-07-04 21:19 2013-10-14 13:08
Reporter carlos View Status public  
Assigned To ajosey
Priority normal Resolution Accepted As Marked  
Status Interpretation Required  
Name Carlos O'Donell
Organization Red Hat
User Reference
Section pthread_rwlock_trywrlock / pthread_rwlock_wrlock (XSH, 3 System Interfaces)
Page Number 1682
Line Number 53998 - 54005
Interp Status Approved
Final Accepted Text Note: 0001767
Summary 0000720: Can pthread_rwlock_wrlock be called recursively by the same thread?
Description In the GNU C Library we have begun implementing support for hardware lock elision in our POSIX thread library. Lock elision brings with it certain restrictions on the behaviour of the interface and limits what locks can or can't be elided. For example because the lock is elided using hardware transactional memory it isn't possible to return EDEADLCK because that would involve taking the lock. This prevents elision for PTHREAD_MUTEX_NORMAL because that mutex type requires deadlock on relock. However, such behaviour is undefined with a PTHREAD_MUTEX_DEFAULT mutex and therefore we can elide all default-style mutex locks in an attempt to increase lock performance.

As part of this work we are looking at the possibility of eliding rwlock locks and we have discovered some inconsistent language.

Lines 54001 to 54005 say:
~~~
The pthread_rwlock_wrlock( ) function shall apply a write lock to the read-write lock referenced by
rwlock. The calling thread acquires the write lock if no other thread (reader or writer) holds the
read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. The calling
thread may deadlock if at the time the call is made it holds the read-write lock (whether a read
or write lock).
~~~

The word "other" in the second sentence appears to imply that the lock may be taken recursively by the same thread without blocking. The use of "other" implying any thread but the present. If the lock can be taken recursively then the option to elide the lock is possible since deadlock detection is entirely optional for a rwlock i.e. "may fail" on line 54021.

However, lines 53998 to 54000 say:
~~~
The pthread_rwlock_trywrlock( ) function shall apply a write lock like the pthread_rwlock_wrlock( )
function, with the exception that the function shall fail if any thread currently holds rwlock (for
reading or writing).
~~~

The word "any" in the first sentence appears to imply that the lock can not be taken recursively. If the lock can't be taken recursively then it must block (deadlock) if a thread already holds the write lock and attempts to take it again. Such a requirement would make it impossible to elide the lock for these calls.

Thus it seems like the language is inconsistent between the two interfaces. Either the write lock can't be taken recursively or it can. Likewise either the word "other" on line 54002 should be removed or the word "other" added after the word "any" on line 53999. Alternatively some other clarification can be provided.
Desired Action One or the other:

(a) No, an rwlock may not be locked recursively.
* Remove the word "other" on line 54002.

(b) Yes, an rwlock may be locked recursively.
* Add the word "other" after "any" on line 53999.

Note:
I am open to the use of more detailed language that avoids the use of the word "other" to imply that the lock may be taken recursively.
Tags tc2-2008
Attached Files

- Relationships
related to 0000722Interpretation Requiredajosey 1003.1(2013)/Issue7+TC1 pthread_rwlock writers preferrence 

-  Notes
(0001664)
andikleen2 (reporter)
2013-07-04 22:21

I would also like to see a clarification how "deadlock" is defined, e.g. with PTHREAD_MUTEX_NORMAL. Does deadlock require stopping on every execution?
(0001665)
geoffclare (manager)
2013-07-05 08:14

pthread_rwlock_trywrlock() fails if any thread holds the lock; it does not deadlock.
(0001666)
Konrad_Schwarz (reporter)
2013-07-05 09:16
edited on: 2013-07-05 11:26

Sorry for misusing this forum for TSX details, but if you were to use the Hardware Lock Elision feature, couldn't you detect a relock attempt, since the word that represents the spin lock is returned as modified locally (globally, it does not change).

Was the decision to not use this feature based on Haswell's limit of 1 nested HLE?

In any case, it seems wrong to make PTHREAD_MUTEX_DEFAULT locks higher performing than PTHREAD_MUTEX_NORMAL, as traditionally PTHREAD_MUTEX_NORMAL locks are the highest performing, and code was optimized to use them. That this code now needs to be revisited (to gain from TSX) is unfortunate.

I think the original, perhaps half-baked, idea for PTHREAD_MUTEX_DEFAULT was to have an easy way of switching between PTHRAD_MUTEX_ERRORCHECK and PTHREAD_MUTEX_NORMAL.

In any case, I think POSIX should re-think its requirement of having PTHREAD_MUTEX_NORMAL deadlock, and turn this into a "may" (perhaps in the next edition), since there is **no** real benefit for applications to have a guaranteed way of deadlocking.

Konrad Schwarz

(0001667)
philip-guenther (reporter)
2013-07-05 09:46

Since pthread_rwlock_unlock()'s spec says this:
   If this function is called to release a write lock for this read-write
   lock object, the read-write lock object shall be put in the unlocked state.

...with no provision for it remaining locked, rwlocks cannot currently be recursive for write locking. I guess that does still open the possibility of returning zero without changing any state in the self-lock case.


My gut reaction is that having self-lock attempts return zero sometimes (only when no contention, right?) will be (a) unexpected for developers, and (b) immensely frustrating to debug, because of the 'sometimes' aspect.


Side note: in OpenBSD, PTHREAD_MUTEX_DEFAULT mutexes call abort() if you pthread_mutex_lock() a mutex you already hold or pthread_mutex_unlock() a mutex you don't hold...and that has turned up several locking bugs in open source projects like glib2, where code *simply failed to grab the right lock* and no one noticed because the default mutexes on Linux returned an ignorable error. The assumption that most code has been debugged completely and that therefore the default behavior should be to turn off the checks and run as fast as possible is a bad joke.
(0001668)
andikleen2 (reporter)
2013-07-05 21:29

Konrad Schwarz:
I don't think this is the right forum to discuss TSX implementation details. I should only note that there are other architectures which support lock elision (e.g. the ones from IBM) that do not have HLE, so for them it would never be an option.
I agree that deadlocks should be all "may" or be sufficiently undefined that the elision behavior is conforming.

Philip Guenther:
"sometimes" behavior is pretty much in the nature of parallelism, threading and locking, there is no way around it.
(0001750)
andikleen3 (reporter)
2013-08-21 23:49

Any decision on this?
Is POSIX rwlocks planning to support lock elision or not?
(0001751)
geoffclare (manager)
2013-08-22 15:43
edited on: 2013-08-27 09:25

As I pointed out in Note: 0001665, the original problem report seems to
be the result of misinterpreting the standard.

The standard is clear that:

pthread_rwlock_[try]rdlock() must be recursive if the calling
thread already holds a read lock.

pthread_rwlock_wrlock() must either deadlock or return an
EDEADLK error if the calling thread already holds a read or
write lock.

pthread_rwlock_trywrlock() must return EBUSY if the calling thread
already holds a read or write lock.

Whether or not the standard should be changed to allow write
locks to be recursive is another matter.

(0001754)
carlos (reporter)
2013-08-23 18:45

Geoff,

This issue is not asking for POSIX to change what is or is not allowed for write locks. That is certainly another issue.

However, it is indeed an issue of interpretation of the standard, and we would like clarification to prevent future misinterpretations.

(1) pthread_rwlock_wrlock

You write:
~~~
pthread_rwlock_wrlock() must either deadlock or return an EDEADLK error if the calling thread already holds a read or write lock.
~~~

Lines 54001 to 54005 say:
~~~
The pthread_rwlock_wrlock( ) function shall apply a write lock to the read-write lock referenced by rwlock. The calling thread acquires the write lock if no other thread (reader or writer) holds the read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock).
~~~

The phrase `if no other thread' appears to have the meaning that the write lock may be taken recursively. That is to say that a single thread may call pthread_rwlock_wrlock as many times as it wishes and the standard allows for an implementation that does not deadlock and instead takes the lock recursively.

If it can't be taken recursively then I'd like the following action item to be taken:

(a) No, an rwlock may not be locked recursively.
* Remove the word "other" on line 54002.
e.g.
Lines 54001 to 54005 would now say:
~~~
The pthread_rwlock_wrlock( ) function shall apply a write lock to the read-write lock referenced by rwlock. The calling thread acquires the write lock if no thread (reader or writer) holds the read-write lock rwlock. Otherwise, the thread shall block until it can acquire the lock. The calling thread may deadlock if at the time the call is made it holds the read-write lock (whether a read or write lock).
~~~

(2) pthread_rwlock_[try]rdlock

You write:
~~~
pthread_rwlock_[try]rdlock() must be recursive if the calling
thread already holds a read lock.

...

pthread_rwlock_wrlock()[SIC] must return EBUSY if the calling thread
already holds a read or write lock.
~~~

I assume you meant to write `pthread_rwlock_[try]rdlock' where you wrote `pthread_rwlock_wrlock()' since the latter should not return EBUSY.

I don't see how these comments are relevant to the question of the behaviour of `pthread_rwlock_trywrlock'.

This issue writes about `pthread_rwlock_trywrlock' in order to make it clear that there is a contradiction in the language used by the standard and that we are seeing clarification.

In summary:
I am looking for clarification on pthread_rwlock_wrlock(). It would appear that the consensus is that it can not be taken recursively and must deadlock or return EDEADLK. In that case I strongly suggest that the word `other' be removed from line 54002 to prevent future confusion.

Andi, several others in the glibc community, and myself have all been confused by the wording of the standard and we would like to see clarification such that we can come to a conclusion on the lock elision work we are doing in glibc.

This has immediate impact on Intel and IBM hardware, so we are interested in a resolution of this issue.
(0001756)
geoffclare (manager)
2013-08-27 09:48

(response to Note: 0001754)

> (1) pthread_rwlock_wrlock
>
> You write:
> ~~~
> pthread_rwlock_wrlock() must either deadlock or return an EDEADLK
> error if the calling thread already holds a read or write lock.
> ~~~
>
> Lines 54001 to 54005 say:
> ~~~
> The pthread_rwlock_wrlock( ) function shall apply a write lock to the
> read-write lock referenced by rwlock. The calling thread acquires the
> write lock if no other thread (reader or writer) holds the read-write
> lock rwlock. Otherwise, the thread shall block until it can acquire
> the lock. The calling thread may deadlock if at the time the call is
> made it holds the read-write lock (whether a read or write lock).
> ~~~
>
> The phrase `if no other thread' appears to have the meaning that the
> write lock may be taken recursively. That is to say that a single
> thread may call pthread_rwlock_wrlock as many times as it wishes and
> the standard allows for an implementation that does not deadlock and
> instead takes the lock recursively.

My take on that paragraph is that this part deals with the possibilities
as regards whether or not other threads hold the lock:

  The pthread_rwlock_wrlock( ) function shall apply a write lock to the
  read-write lock referenced by rwlock. The calling thread acquires the
  write lock if no other thread (reader or writer) holds the read-write
  lock rwlock. Otherwise, the thread shall block until it can acquire
  the lock.

and this part deals (together with the EDEADLK error) with the
possibilities as regards whether or not the calling thread already holds
the lock:

  The calling thread may deadlock if at the time the call is
  made it holds the read-write lock (whether a read or write lock).

If you treat the first part as also applying when the calling thread
holds the lock, then the "Otherwise" clause makes no sense as it would
mandate deadlock for this case whereas the second part makes it optional.

Also, if recursive write locks were allowed there would be a
requirement, as there is for read locks, that "the application shall
ensure that the thread performs matching unlocks", and the requirements
for pthread_rwlock_trywrlock() would be different.

> If it can't be taken recursively then I'd like the following action item to be taken:
>
> (a) No, an rwlock may not be locked recursively.
> * Remove the word "other" on line 54002.
> e.g.
> Lines 54001 to 54005 would now say:
> ~~~
> The pthread_rwlock_wrlock( ) function shall apply a write lock to the
> read-write lock referenced by rwlock. The calling thread acquires the
> write lock if no thread (reader or writer) holds the read-write lock
> rwlock. Otherwise, the thread shall block until it can acquire the
> lock. The calling thread may deadlock if at the time the call is made
> it holds the read-write lock (whether a read or write lock).
> ~~~

That still wouldn't be quite right. The "Otherwise" clause also needs to
change, to:

    Otherwise, if another thread holds the read-write lock rwlock,
    the calling thread shall block until it can acquire the lock.

in order not to conflict with the final sentence.

> (2) pthread_rwlock_[try]rdlock
>
> You write:
> ~~~
> pthread_rwlock_[try]rdlock() must be recursive if the calling
> thread already holds a read lock.
>
> ...
>
> pthread_rwlock_wrlock()[SIC] must return EBUSY if the calling thread
> already holds a read or write lock.
> ~~~
>
> I assume you meant to write `pthread_rwlock_[try]rdlock' where you wrote `pthread_rwlock_wrlock()' since the latter should not return EBUSY.
>

No, I meant to write pthread_rwlock_trywrlock(). I have corrected the
comment.
(0001757)
torvald (reporter)
2013-08-28 08:12
edited on: 2013-08-28 08:13

Geoff, you write in Note: 0001756

> [...] this part deals (together with the EDEADLK error) with the
> possibilities as regards whether or not the calling thread already holds
> the lock:
>
> The calling thread may deadlock if at the time the call is
> made it holds the read-write lock (whether a read or write lock).

To me this seems to allow deadlocks, but not require them (i.e., it may be a recursive write lock, or may not be). However, ...

> If you treat the first part as also applying when the calling thread
> holds the lock, then the "Otherwise" clause makes no sense as it would
> mandate deadlock for this case whereas the second part makes it optional.
>
> Also, if recursive write locks were allowed there would be a
> requirement, as there is for read locks, that "the application shall
> ensure that the thread performs matching unlocks", and the requirements
> for pthread_rwlock_trywrlock() would be different.

... you seem to understand this sentence as requiring deadlocks (so it is not a recursive write lock). What is the correct interpretation? If implementations may choose to implement a recursive writelock for pthread_rwlock_wrlock, then for consistency reasons, I suppose it should be clarified that pthread_rwlock_trywrlock *may* return EBUSY, or may acquire recursively.

Otherwise, if pthread_rwlock_wrlock is not recursive, then what about rephrasing as such:

  The calling thread acquires the write lock if no other thread (reader or
  writer) holds the read-write lock rwlock; otherwise, the thread shall block
  until it can acquire the lock. The calling thread shall deadlock if at the
  time the call is made it holds the read-write lock (whether a read or write
  lock).

This would clarify the deadlock requirement. Also, it would clarify the context of "otherwise" by linking the first and second sentence with ";" (this is something we could apply irrespective of we resolve the issues around the last sentence).

(0001759)
geoffclare (manager)
2013-08-28 09:24
edited on: 2013-08-28 14:01

(response to Note: 0001757)

I am sure that the intention is that pthread_rwlock_wrlock() must
either deadlock or return EDEADLK when the calling thread already holds
the lock. As I said in my previous note, if recursive locks were
allowed, there would be a requirement about the need for matching
unlocks, and the requirements for pthread_rwlock_trywrlock() would be
different (it would be worded like pthread_rwlock_tryrdlock()). See
also Philip Guenther's point (Note: 0001667) about pthread_rwlock_unlock().

The suggested text that says "The calling thread shall deadlock if at
the time the call is made it holds the read-write lock" would imply
that an EDEADLK error is not allowed, contradicting the ERRORS
section. I think removing "other" and altering the "Otherwise" clause
as per my previous note would suffice to fix the loophole that appears
to allow pthread_rwlock_wrlock() to be recursive.

(0001761)
torvald (reporter)
2013-08-28 12:16

I agree about a plain "shall deadlock" disallowing the error code. Instead, what about saying "shall either deadlock or return an EDEADLK"? The plain "may deadlock" seems to have caused quite a bit of confusion, so perhaps it makes sense to be more verbose here.
(0001762)
geoffclare (manager)
2013-08-28 13:54

I like the "either ... or ..." suggestion, and suggest making it cover
other deadlock conditions as well...

Change:

    The calling thread acquires the write lock if no other thread
    (reader or writer) holds the read-write lock rwlock. Otherwise,
    the thread shall block until it can acquire the lock. The calling
    thread may deadlock if at the time the call is made it holds the
    read-write lock (whether a read or write lock).

to:

    The calling thread shall acquire the write lock if no thread
    (reader or writer) holds the read-write lock rwlock. Otherwise, if
    another thread holds the read-write lock rwlock, the calling
    thread shall block until it can acquire the lock. If a deadlock
    condition occurs or the calling thread already owns the read-write
    lock for writing or reading, the call shall either deadlock or
    return EDEADLK.
(0001767)
geoffclare (manager)
2013-08-29 09:08
edited on: 2013-08-29 15:09

Interpretation response
------------------------
The standard clearly states that reader-writer locks held for reading
are recursive and that reader-writer locks held for writing are not
recursive, and conforming implementations must conform to this.

Rationale:
-------------
It is clear from the description of pthread_rwlock_[try]rdlock()
and pthread_rwlock_unlock() that read locks are recursive.
It is clear from the description of pthread_rwlock_trywrlock()
and pthread_rwlock_unlock() that write locks are not recursive.
The intention is that pthread_rwlock_wrlock() either deadlocks
or returns EDEADLK if the caller already holds a write lock, but
this is written as two separate "may" clauses (one in the
DESCRIPTION and one in the ERRORS section), which creates a
loophole that allows pthread_rwlock_wrlock() to succeed (as a
no-op). However, this behavior would be of no use to
applications. In particular, if an application tried to make
matching pthread_rwlock_unlock() calls for two successful
pthread_rwlock_wrlock() calls, the first unlock call would set
the state to unlocked and the second unlock call would result in
undefined behavior.

Notes to the Editor (not part of this interpretation):
-------------------------------------------------------
Make the change in Note: 0001762.

(0001771)
carlos (reporter)
2013-08-29 15:07

I'm happy with the new wording suggested in Note: 0001762.

The goal here is to clarify the intent of the standard in order to allow an implementation to use elision or not use elision for locks of that particular type.
(0001804)
ajosey (manager)
2013-09-06 04:53

Interpretation Proposed 6 Sep 2013
(0001896)
ajosey (manager)
2013-10-14 13:08

Interpretation approved 14 October 2013

- Issue History
Date Modified Username Field Change
2013-07-04 21:19 carlos New Issue
2013-07-04 21:19 carlos Status New => Under Review
2013-07-04 21:19 carlos Assigned To => ajosey
2013-07-04 21:19 carlos Name => Carlos O'Donell
2013-07-04 21:19 carlos Organization => Red Hat
2013-07-04 21:19 carlos Section => pthread_rwlock_trywrlock / pthread_rwlock_wrlock (XSH, 3 System Interfaces)
2013-07-04 21:19 carlos Page Number => 1682
2013-07-04 21:19 carlos Line Number => 53998 - 54005
2013-07-04 22:21 andikleen2 Note Added: 0001664
2013-07-04 22:21 andikleen2 Issue Monitored: andikleen2
2013-07-05 08:14 geoffclare Note Added: 0001665
2013-07-05 09:16 Konrad_Schwarz Note Added: 0001666
2013-07-05 09:46 philip-guenther Note Added: 0001667
2013-07-05 11:26 Konrad_Schwarz Note Edited: 0001666
2013-07-05 21:29 andikleen2 Note Added: 0001668
2013-07-18 16:06 eblake Relationship added related to 0000722
2013-08-21 23:49 andikleen3 Note Added: 0001750
2013-08-22 15:43 geoffclare Note Added: 0001751
2013-08-23 18:45 carlos Note Added: 0001754
2013-08-27 09:25 geoffclare Note Edited: 0001751
2013-08-27 09:48 geoffclare Note Added: 0001756
2013-08-28 08:12 torvald Note Added: 0001757
2013-08-28 08:13 torvald Note Edited: 0001757
2013-08-28 09:24 geoffclare Note Added: 0001759
2013-08-28 12:16 torvald Note Added: 0001761
2013-08-28 13:54 geoffclare Note Added: 0001762
2013-08-28 14:01 geoffclare Note Edited: 0001759
2013-08-29 09:08 geoffclare Note Added: 0001767
2013-08-29 09:10 geoffclare Note Edited: 0001767
2013-08-29 15:07 carlos Note Added: 0001771
2013-08-29 15:09 geoffclare Interp Status => Pending
2013-08-29 15:09 geoffclare Final Accepted Text => Note: 0001767
2013-08-29 15:09 geoffclare Status Under Review => Interpretation Required
2013-08-29 15:09 geoffclare Resolution Open => Accepted As Marked
2013-08-29 15:09 geoffclare Note Edited: 0001767
2013-08-29 15:09 geoffclare Tag Attached: tc2-2008
2013-09-06 04:53 ajosey Interp Status Pending => Proposed
2013-09-06 04:53 ajosey Note Added: 0001804
2013-10-14 13:08 ajosey Interp Status Proposed => Approved
2013-10-14 13:08 ajosey Note Added: 0001896


Mantis 1.1.6[^]
Copyright © 2000 - 2008 Mantis Group
Powered by Mantis Bugtracker