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

[Chad Versace]

Quoting Emil Velikov (2015-03-25 07:30:00)
> On 24 March 2015 at 17:37, Jose Fonseca <jfonseca at vmware.com> wrote:
> > Is wcore_display_teardown called only once, or when destroying each display?
> >
> > If the latter, then it's not safe to call `mtx_destroy(&mutex)` on
> > `wcore_display_teardown`.  Otherwise when  wcore_display_init is called
> > next, wcore_display_init_once() will not be called a 2nd time, hence mutex
> > will stay invalid.
> >
> >
> > This should probably done at waffle_teardown.

Right. The mutex should be destroyed, if anywhere, in waffle_teardown().
But, we should probably not destroy it at all, because of my next
suggestion...

> > BTW, if the mutex was initialized at waffle_init, then once_flag wouldn't
> > even be necessary.

Technically correct, but I eventually want to deprecate waffle_init(),
moving its platform parameter to a new waffle_display_connect()
function. Using a once_flag in wcore_display_init() fits better with
that longterm goal. Let's keep Emil's once_flag in this patch.

If use a once_flag, then I believe there is no safe place to
destroy the mutex, because we can't re-initialize the mutex by calling
wcore_display_init_once() a second time.

Which leads to the question: Emil, what benefit do you expect from
destroying the mutex? If Waffle were continuously creating mutexes, then
Waffle would need to destroy them too to prevent leaks. But Waffle only
creates a small, fixed number of global mutexes during the process's
lifetime. And "leaking" them doesn't lead to ongoing memory loss.
Moreover, with pthreads... see my comments below.

> 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.

> Might be worth looking into, once I've got waffle_teardown() hooked in there.
> 
> -Emil
> _______________________________________________
> waffle mailing list
> waffle at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/waffle