[webkit-reviews] review denied: [Bug 135833] Clean up GMutexLocker : [Attachment 236434] Patch

[bugzilla-daemon at webkit.org]

Carlos Garcia Campos <cgarcia at igalia.com> has denied Zan Dobersek
<zandobersek at gmail.com>'s request for review:
Bug 135833: Clean up GMutexLocker
https://bugs.webkit.org/show_bug.cgi?id=135833

Attachment 236434: Patch
https://bugs.webkit.org/attachment.cgi?id=236434&action=review

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=236434&action=review


> Source/WTF/wtf/gobject/GMutexLocker.h:28
> -namespace WebCore {
> +namespace WTF {

wow!

> Source/WebCore/platform/graphics/gstreamer/VideoSinkGStreamer.cpp:237
> -    if (priv->bufferMutex) {
> -	   g_mutex_clear(priv->bufferMutex);
> -	   delete priv->bufferMutex;
> -	   priv->bufferMutex = 0;
> -    }
> +    g_mutex_clear(&priv->bufferMutex);

You can't do this here, dispose might be called multiple times, and
g_mutex_clear should not be used on an already cleared mutex, because the
internal pointer is not checked and it's not reset either when freed. Since we
are using placement new syntax, I would add a constructor of
WebKitVideoSinkPrivate to init the mutex, and a destructor to clear it.