Quoting Emil Velikov (2015-03-25 07:30:00) > On 24 March 2015 at 17:37, Jose Fonseca <jfons...@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@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/waffle _______________________________________________ waffle mailing list waffle@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/waffle