On 26/03/15 14:50, Chad Versace wrote:
Quoting Emil Velikov (2015-03-25 07:30:00)
On 24 March 2015 at 17:37, Jose Fonseca <[email protected]> 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.
I think it's fine to let it leak on destroy, but just FYI on Windows
mutex (ie, CriticalSections) may allocate/free memory on certain
configurations. IIRC, they can allocate memory for stack backtraces,
for debugging purposes.
The approach we use to statically initialize critical sections on
windows is unofficial. Read
http://locklessinc.com/articles/pthreads_on_windows/ for the gritty details.
Jose
_______________________________________________
waffle mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/waffle
