|
From: Mateusz J. <m.j...@is...> - 2016-10-06 12:57:22
|
Hello, I'm using valgrind built from SVN, r16025. I encountered helgrind complaining about invalid argument used with pthread_rwlock_destroy, for variable initialized with PTHREAD_RWLOCK_INITIALIZER. Is this a known thing? Am I doing something wrong? I searched the mailing list archives on https://sourceforge.net/p/valgrind/mailman/valgrind-users/, but couldn't find anything. I'm asking because I think pthreads allows me to destroy rwlock initialized that way. I reproduced this with minimal test case on Ubuntu 14.04: $ lsb_release -a No LSB modules are available. Distributor ID: Ubuntu Description: Ubuntu 14.04.5 LTS Release: 14.04 Codename: trusty $ cat pthread_rwlock_destroy.c # define _POSIX_C_SOURCE 20161006L #include <assert.h> #include <pthread.h> int main(void) { pthread_rwlock_t l = PTHREAD_RWLOCK_INITIALIZER; assert(0 == pthread_rwlock_destroy(&l)); return 0; } $ gcc -Wall -Wextra -std=c99 -pedantic -O0 -ggdb pthread_rwlock_destroy.c -pthread $ valgrind --tool=helgrind ./a.out ==4199== Helgrind, a thread error detector ==4199== Copyright (C) 2007-2015, and GNU GPL'd, by OpenWorks LLP et al. ==4199== Using Valgrind-3.12.0.SVN and LibVEX; rerun with -h for copyright info ==4199== Command: ./a.out ==4199== ==4199== ---Thread-Announcement------------------------------------------ ==4199== ==4199== Thread #1 is the program's root thread ==4199== ==4199== ---------------------------------------------------------------- ==4199== ==4199== Thread #1: pthread_rwlock_destroy with invalid argument ==4199== at 0x4C302D6: pthread_rwlock_destroy_WRK (hg_intercepts.c:2096) ==4199== by 0x4C33018: pthread_rwlock_destroy (hg_intercepts.c:2113) ==4199== by 0x400707: main (pthread_rwlock_destroy.c:9) ==4199== ==4199== ==4199== For counts of detected and suppressed errors, rerun with: -v ==4199== Use --history-level=approx or =none to gain increased speed, at ==4199== the cost of reduced accuracy of conflicting-access information ==4199== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0) Regards, Mateusz |
|
From: Alex B. <al...@al...> - 2016-10-06 14:16:27
|
> On 6 Oct 2016, at 13:43, Mateusz Jemielity <m.j...@is...> wrote: > > I'm using valgrind built from SVN, r16025. I encountered helgrind > complaining about invalid argument used with pthread_rwlock_destroy, for > variable initialized with PTHREAD_RWLOCK_INITIALIZER. Is this a known thing? > Am I doing something wrong? I searched the mailing list archives on > https://sourceforge.net/p/valgrind/mailman/valgrind-users/, but couldn't > find anything. I'm asking because I think pthreads allows me to destroy > rwlock initialized that way. > > I reproduced this with minimal test case on Ubuntu 14.04: > $ lsb_release -a > No LSB modules are available. > Distributor ID: Ubuntu > Description: Ubuntu 14.04.5 LTS > Release: 14.04 > Codename: trusty > $ cat pthread_rwlock_destroy.c > # define _POSIX_C_SOURCE 20161006L > > #include <assert.h> > #include <pthread.h> > > int main(void) > { > pthread_rwlock_t l = PTHREAD_RWLOCK_INITIALIZER; > assert(0 == pthread_rwlock_destroy(&l)); > return 0; > } According to the man page: > The pthread_rwlock_destroy() function is used to destroy a read/write lock previously created with pthread_rwlock_init(). You did not create the lock with pthread_rwload_init() - you used a static initialisation. Therefore my understanding is that you do not need to (and should not) call pthread_rwlock_destroy(). -- Alex Bligh |
|
From: Mateusz J. <m.j...@is...> - 2016-10-06 15:06:50
|
>> On 6 Oct 2016, at 13:43, Mateusz Jemielity <m.j...@is...> wrote: >> >> I'm using valgrind built from SVN, r16025. I encountered helgrind >> complaining about invalid argument used with pthread_rwlock_destroy, >> for variable initialized with PTHREAD_RWLOCK_INITIALIZER. Is this a known thing? >> Am I doing something wrong? I searched the mailing list archives on >> https://sourceforge.net/p/valgrind/mailman/valgrind-users/, but >> couldn't find anything. I'm asking because I think pthreads allows me >> to destroy rwlock initialized that way. >> >> I reproduced this with minimal test case on Ubuntu 14.04: >> $ lsb_release -a >> No LSB modules are available. >> Distributor ID:IDUbuntu >> Description:DescriptionUbuntu 14.04.5 LTS >> Release:Release14.04 >> Codename:Codenametrusty >> $ cat pthread_rwlock_destroy.c >> # define _POSIX_C_SOURCE 20161006L >> >> #include <assert.h> >> #include <pthread.h> >> >> int main(void) >> { >> pthread_rwlock_t l = PTHREAD_RWLOCK_INITIALIZER; >> assert(0 == pthread_rwlock_destroy(&l)); return 0; } > >According to the man page: > >> The pthread_rwlock_destroy() function is used to destroy a read/write lock previously created with pthread_rwlock_init(). > >You did not create the lock with pthread_rwload_init() - you used a static initialisation. Therefore my understanding is that you do not need to (and should not) call pthread_rwlock_destroy(). > >-- >Alex Bligh > Docs at http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_des troy.html say: In cases where default read-write lock attributes are appropriate, the macro PTHREAD_RWLOCK_INITIALIZER can be used to initialize read-write locks. The effect shall be equivalent to dynamic initialization by a call to pthread_rwlock_init() with the attr parameter specified as NULL, except that no error checks are performed. The effects are equivalent to pthread_rwlock_init, thus in my opinion they require pthread_rwlock_destroy. The internals of rwlock are opaque to me and I don't know what resources are used (especially on different systems and implementations), so I should cleanly free them just in case. I know it's not common usecase to do something like that. In my scenario I want to replace rwlock created using static initializer with another one that uses custom attributes. - Mateusz |
|
From: Alex B. <al...@al...> - 2016-10-06 15:53:19
|
> On 6 Oct 2016, at 16:06, Mateusz Jemielity <m.j...@is...> wrote: > > The effects are equivalent to pthread_rwlock_init, thus in my opinion they > require pthread_rwlock_destroy. The internals of rwlock are opaque to me and > I don't know what resources are used (especially on different systems and > implementations), so I should cleanly free them just in case. > I know it's not common usecase to do something like that. In my scenario I > want to replace rwlock created using static initializer with another one > that uses custom attributes. I can't say I've used pthread_rwlock_* much but I've used pthread_mutex_* before which has the same init/destroy/static initializer trio and that seemed right. However, I'm not sure why you'd need to call pthread_*_destroy on a statically initialised object, given that presumably you destroy it when the program is about to exit and the resources would be given back to the OS anyway. -- Alex Bligh |
|
From: Patrick J. L. <lop...@gm...> - 2016-10-06 20:27:10
|
On Thu, Oct 6, 2016 at 8:53 AM, Alex Bligh <al...@al...> wrote: > > > > On 6 Oct 2016, at 16:06, Mateusz Jemielity <m.j...@is...> wrote: > > > > The effects are equivalent to pthread_rwlock_init, thus in my opinion they > > require pthread_rwlock_destroy. > > However, I'm not sure why you'd need to call pthread_*_destroy on a statically > initialised object, given that presumably you destroy it when the program > is about to exit and the resources would be given back to the OS anyway. In this example it is in main(), but there is no reason the same sequence could not appear in the middle of a long-running program, in which case failing to call pthread_rwlock_destroy() could potentially leak resources. I agree with Mateusz. POSIX clearly requires a call to pthread_rwlock_destroy() on RW locks initialized via PTHREAD_RW_INITIALIZER to ensure proper freeing of resources. - Pat |
|
From: Ivo R. <iv...@iv...> - 2016-10-07 18:04:12
|
2016-10-06 22:27 GMT+02:00 Patrick J. LoPresti <lop...@gm...>: > On Thu, Oct 6, 2016 at 8:53 AM, Alex Bligh <al...@al...> wrote: > > > > > > > On 6 Oct 2016, at 16:06, Mateusz Jemielity < > m.j...@is...> wrote: > > > > > > The effects are equivalent to pthread_rwlock_init, thus in my opinion > they > > > require pthread_rwlock_destroy. > > > > However, I'm not sure why you'd need to call pthread_*_destroy on a > statically > > initialised object, given that presumably you destroy it when the program > > is about to exit and the resources would be given back to the OS anyway. > > In this example it is in main(), but there is no reason the same sequence > could not appear in the middle of a long-running program, in which case > failing to call pthread_rwlock_destroy() could potentially leak resources. > > I agree with Mateusz. POSIX clearly requires a call to > pthread_rwlock_destroy() on RW locks initialized via PTHREAD_RW_INITIALIZER > to ensure proper freeing of resources. > Indeed, POSIX [1] explicitly says that PTHREAD_RWLOCK_INITIALIZER shall be equivalent to dynamic initialization by a call to pthread_rwlock_init(). However our tools (DRD, Helgrind) cannot detect such implicitly initialized primitive easily. Simply because there is no pthread_rwlock_init() called for them. For example on Solaris, PTHREAD_RWLOCK_INITIALIZER is a macro which expands to pthread_rwlock_t with all fields set as if pthread_rwlock_init() was called instead. I don't see an efficient way how to fix such false positives. I. [1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_rwlock_destroy.html |