Message ID | 1477083990-9983-1-git-send-email-tuliom@linux.vnet.ibm.com |
---|---|
State | New |
Headers | show |
On Fri, 2016-10-21 at 19:06 -0200, Tulio Magno Quites Machado Filho wrote: > Explain that pthread_rwlock_unlock may crash if called on a lock not > held by the current thread. > > 2016-10-21 Tulio Magno Quites Machado Filho <tuliom@linux.vnet.ibm.com> > > * nptl/pthread_rwlock_unlock.c: Add a comment explaining its > behavior when eliding a lock not held by the current thread. > * sysdeps/powerpc/nptl/elide.h: Likewise. > --- > nptl/pthread_rwlock_unlock.c | 4 ++++ > sysdeps/powerpc/nptl/elide.h | 2 ++ > 2 files changed, 6 insertions(+) > > diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c > index a6cadd4..112f748 100644 > --- a/nptl/pthread_rwlock_unlock.c > +++ b/nptl/pthread_rwlock_unlock.c > @@ -35,6 +35,10 @@ __pthread_rwlock_unlock (pthread_rwlock_t *rwlock) > > LIBC_PROBE (rwlock_unlock, 1, rwlock); > > + /* Trying to elide an unlocked lock may crash the process. This > + is expected and is compatible with POSIX.1-2008: "results are > + undefined if the read-write lock rwlock is not held by the > + calling thread". */ > if (ELIDE_UNLOCK (rwlock->__data.__writer == 0 > && rwlock->__data.__nr_readers == 0)) > return 0; > diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h > index 77bd82e..c573981 100644 > --- a/sysdeps/powerpc/nptl/elide.h > +++ b/sysdeps/powerpc/nptl/elide.h > @@ -102,6 +102,8 @@ __elide_unlock (int is_lock_free) > { > if (is_lock_free) > { > + /* Intentionally crashes when trying to unlock a lock not held by this > + thread. */ I'd rather say that this can happen, and is okay (refering to the comment in pthread_rwlock_unlock.c). Otherwise, LGTM. > __libc_tend (0); > return true; > }
Torvald Riegel <triegel@redhat.com> writes: > On Fri, 2016-10-21 at 19:06 -0200, Tulio Magno Quites Machado Filho > wrote: >> diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h >> index 77bd82e..c573981 100644 >> --- a/sysdeps/powerpc/nptl/elide.h >> +++ b/sysdeps/powerpc/nptl/elide.h >> @@ -102,6 +102,8 @@ __elide_unlock (int is_lock_free) >> { >> if (is_lock_free) >> { >> + /* Intentionally crashes when trying to unlock a lock not held by this >> + thread. */ > > I'd rather say that this can happen, and is okay (refering to the > comment in pthread_rwlock_unlock.c). Otherwise, LGTM. Good point. I'll make this change before pushing it. Thanks!
diff --git a/nptl/pthread_rwlock_unlock.c b/nptl/pthread_rwlock_unlock.c index a6cadd4..112f748 100644 --- a/nptl/pthread_rwlock_unlock.c +++ b/nptl/pthread_rwlock_unlock.c @@ -35,6 +35,10 @@ __pthread_rwlock_unlock (pthread_rwlock_t *rwlock) LIBC_PROBE (rwlock_unlock, 1, rwlock); + /* Trying to elide an unlocked lock may crash the process. This + is expected and is compatible with POSIX.1-2008: "results are + undefined if the read-write lock rwlock is not held by the + calling thread". */ if (ELIDE_UNLOCK (rwlock->__data.__writer == 0 && rwlock->__data.__nr_readers == 0)) return 0; diff --git a/sysdeps/powerpc/nptl/elide.h b/sysdeps/powerpc/nptl/elide.h index 77bd82e..c573981 100644 --- a/sysdeps/powerpc/nptl/elide.h +++ b/sysdeps/powerpc/nptl/elide.h @@ -102,6 +102,8 @@ __elide_unlock (int is_lock_free) { if (is_lock_free) { + /* Intentionally crashes when trying to unlock a lock not held by this + thread. */ __libc_tend (0); return true; }