Bug 135833 - Clean up GMutexLocker
Summary: Clean up GMutexLocker
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Zan Dobersek
URL:
Keywords:
Depends on:
Blocks: 135800
  Show dependency treegraph
 
Reported: 2014-08-12 02:19 PDT by Zan Dobersek
Modified: 2014-08-12 03:39 PDT (History)
2 users (show)

See Also:


Attachments
Patch (14.45 KB, patch)
2014-08-12 02:25 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff
Patch (14.76 KB, patch)
2014-08-12 03:13 PDT, Zan Dobersek
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Zan Dobersek 2014-08-12 02:19:01 PDT
Clean up GMutexLocker
Comment 1 Zan Dobersek 2014-08-12 02:25:32 PDT
Created attachment 236434 [details]
Patch
Comment 2 Carlos Garcia Campos 2014-08-12 02:55:08 PDT
Comment on attachment 236434 [details]
Patch

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.
Comment 3 Zan Dobersek 2014-08-12 03:13:12 PDT
Created attachment 236436 [details]
Patch
Comment 4 Zan Dobersek 2014-08-12 03:39:35 PDT
Comment on attachment 236436 [details]
Patch

Clearing flags on attachment: 236436

Committed r172441: <http://trac.webkit.org/changeset/172441>
Comment 5 Zan Dobersek 2014-08-12 03:39:42 PDT
All reviewed patches have been landed.  Closing bug.