[waffle] [PATCH v2 2/3] wcore: rework non-compatible mutex initialization

[Emil Velikov]

On 27 March 2015 at 16:09, Chad Versace <chad.versace at intel.com> wrote:
> Quoting Emil Velikov (2015-03-26 19:24:21)
>> On 26 March 2015 at 14:50, Chad Versace <chad.versace at intel.com> 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