Index | Thread | Search

From:
Ingo Schwarze <schwarze@usta.de>
Subject:
Re: pthread_rwlock_combo
To:
Ted Unangst <tedu@tedunangst.com>
Cc:
tech@openbsd.org
Date:
Fri, 4 Jul 2025 20:27:01 +0200

Download raw body.

Thread
Hello Ted,

Ted Unangst wrote on Thu, Jul 03, 2025 at 02:53:55PM -0400:

> I think the rwlock pages are also ripe for collection.
> These are all Copyright (c) 1998 Alex Nash.

OK schwarze@.

> I reworked some of the wording so that that the timed and try lock
> functions are together.  I think the main body of description is
> easier to follow this way.

Sounds good.

> The errors section could probably use another pass.  I favored being
> specific, but there are perhaps more lists than ideal.

Doesn't seem that bad to me.  It's prpobably best to get this in
and not digress into patch ping pong.  If we want, we can polish
the ERRORS section later.

> Also some changes to make the language more direct, i.e.:
> -function is used to initialize a read/write lock, with attributes
> +function initializes a read/write lock, with attributes

Sure.


I realize you did not add the following sentence:

     The results of acquiring a read lock while the calling thread
     holds a write lock are undefined.

That sentence is already in pthread_rwlock_rdlock(3), but it
contradicts our own manual page, which says:

     The pthread_rwlock_rdlock(), ... functions fail if:
     [EDEADLK]   The current thread already owns lock for writing.

That behaviour is also required by POSIX.

Similarly, you did not add

     The results are undefined if the calling thread already holds
     the lock at the time the call is made.

which is already in pthread_rwlock_wrlock(3), but it contradicts:

     The pthread_rwlock_wrlock(), ... functions fail if:
     [EDEADLK]   The calling thread already owns the read/write
                 lock (for reading or writing).

Again, that behaviour is required by POSIX.

Looking at our code, my impression is that this EDEADLK behaviour
is described correctly and we conform to POSIX in this respect,
so i think you should simply delete the two lies about undefined
behaviour.  I might misread the code though, in particular given
that /usr/src/lib/librthread/rthread_rwlock.c and
/usr/src/lib/librthread/rthread_rwlock_compat.c both exist.


If you want, you can re-use the simpler RETURN VALUES section from
pthread_cond_init(3):

     These functions return zero for success and positive error
     numbers for failure.

In think you should add ".Xr pthreads 3" to SEE ALSO.

Below HISTORY, the "first appeared in FreeBSD 3.0" might be a lie -
or are the functions really FreeBSD inventions?  Also, you lost part
of the OpenBSD history.  I'd suggest dropping the FreeBSD history
and simply saying:

.Fn pthread_rwlock_init ,
.Fn pthread_rwlock_destroy ,
.Fn pthread_rwlock_rdlock ,
.Fn pthread_rwlock_tryrdlock ,
.Fn pthread_rwlock_wrlock ,
.Fn pthread_rwlock_trywrlock ,
and
.Fn pthread_rwlock_unlock
have been avaliable since
.Ox 2.5 ,
and
.Fn pthread_rwlock_timedrdlock
and
.Fn pthread_rwlock_timedwrlock
since
.Ox 4.8 .

The STANDARDS section should probably be updated, -susv2 sounds
unusually old-fashioned, but no need to mix that into this diff.

Yours,
  Ingo