On 27 March 2015 at 16:09, Chad Versace <[email protected]> wrote: > Quoting Emil Velikov (2015-03-26 19:24:21) >> On 26 March 2015 at 14:50, Chad Versace <[email protected]> wrote: >> > Quoting Emil Velikov (2015-03-25 07:30:00) >> >> Indeed you're bang on the spot. id_counter should be locked throughout >> >> a series of display_{connect, disconnect}, thus we should push >> >> mtx_{init,destroy} up to waffle_{init,teardown}. What worries me is >> >> that none of the tests (ran in valgrind) point out any issues. >> > >> > I could be wrong, but I believe pthread_mutex_create/destroy don't >> > allocate/free memory. They just initialize/reset the fields in the >> > pthread_mutex_t struct. (Otherwise, how would PTHREAD_MUTEX_INITIALIZER >> > work?). That may explain why you didn't see the expected Valgrind >> > errors, because no allocation took place. >> > >> I was thinking about thread 2 diving into display_connect() and >> attempting to lock a destroyed mutex. Afaics in such case the function >> will return EINVAL, which I naively assumed that is a undefined >> behaviour. Although with the failed locking around id_counter I assume >> another problem could have happened. > > Not a naive assumption. POSIX does define that undefined behavior occurs > if you try to lock an uninitialized or destroyed mutex. From > the pthread_mutex_lock(3p) manpage: > > If mutex does not refer to an initialized mutex object, the behavior of > pthread_mutex_lock(), pthread_mutex_trylock(), and > pthread_mutex_unlock() is undefined. Thanks Chad !
I knew I wasn't imagining stuff, yet a quick internet search [1] pointed out that EINVAL is returned. Coming back to the patch in question - I'm ok with doing the call_once/mtx_init dance at display_connect and leaking the mutex. Will send out updated patch in a bit. Cheers, Emil [1] http://pubs.opengroup.org/onlinepubs/007908799/xsh/pthread_mutex_lock.html _______________________________________________ waffle mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/waffle
