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

[Jose Fonseca]

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.

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


Jose



On 24/03/15 16:56, Emil Velikov wrote:
> C11 does not specify a static initializer, based on the idea that the
> a mutex will be platform and/or implementation dependent. As such the
> alternative solution is to initialize the mutex with call_once/mtx_init.
> This will allow us to remove the transition hack, and in due time move
> to the system's C11 threads implementation.
>
> v2: Actually use call_once() to prevent the possibility of multiple
> threads hitting the mtx_init() at the same time. Suggested by Jose.
>
> Cc: Jose Fonseca <jfonseca at vmware.com>
> Signed-off-by: Emil Velikov <emil.l.velikov at gmail.com>
> ---
>   src/waffle/core/wcore_display.c | 19 ++++++++++++++++++-
>   src/waffle/core/wcore_display.h |  9 ++-------
>   2 files changed, 20 insertions(+), 8 deletions(-)
>
> diff --git a/src/waffle/core/wcore_display.c b/src/waffle/core/wcore_display.c
> index 2feeeba..2cf0dcd 100644
> --- a/src/waffle/core/wcore_display.c
> +++ b/src/waffle/core/wcore_display.c
> @@ -29,16 +29,25 @@
>
>   #include "wcore_display.h"
>
> +static mtx_t mutex;
> +
> +static void
> +wcore_display_init_once(void)
> +{
> +    mtx_init(&mutex, mtx_plain);
> +}
> +
>   bool
>   wcore_display_init(struct wcore_display *self,
>                      struct wcore_platform *platform)
>   {
>       static size_t id_counter = 0;
> -    static mtx_t mutex = _MTX_INITIALIZER_NP;
> +    static once_flag flag = ONCE_FLAG_INIT;
>
>       assert(self);
>       assert(platform);
>
> +    call_once(&flag, wcore_display_init_once);
>       mtx_lock(&mutex);
>       self->api.display_id = ++id_counter;
>       mtx_unlock(&mutex);
> @@ -52,3 +61,11 @@ wcore_display_init(struct wcore_display *self,
>
>       return true;
>   }
> +
> +bool
> +wcore_display_teardown(struct wcore_display *self)
> +{
> +    assert(self);
> +    mtx_destroy(&mutex);
> +    return true;
> +}
> diff --git a/src/waffle/core/wcore_display.h b/src/waffle/core/wcore_display.h
> index de6ca5e..6d95d98 100644
> --- a/src/waffle/core/wcore_display.h
> +++ b/src/waffle/core/wcore_display.h
> @@ -56,10 +56,5 @@ wcore_display_init(struct wcore_display *self,
>                      struct wcore_platform *platform);
>
>
> -static inline bool
> -wcore_display_teardown(struct wcore_display *self)
> -{
> -    (void) self;
> -    assert(self);
> -    return true;
> -}
> +bool
> +wcore_display_teardown(struct wcore_display *self);
>