[Webkit-unassigned] [Bug 115352] [gstreamer] Make sure gstreamer source element is thread-safe

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=115352





--- Comment #36 from Andre Moreira Magalhaes <andrunko at gmail.com>  2013-08-23 08:21:18 PST ---
Created an attachment (id=209462)
 --> (https://bugs.webkit.org/attachment.cgi?id=209462&action=review)
GMutexLocker.h attempt

(In reply to comment #35)
> (In reply to comment #34)
> > (From update of attachment 209131 [details] [details])
> > View in context: https://bugs.webkit.org/attachment.cgi?id=209131&action=review
> > 
> > > Source/WTF/wtf/gobject/GMutexLocker.h:39
> > > +    inline GMutexLocker()
> > > +        : m_mutex(0)
> > > +    {
> > > +    }
> > 
> > This is unused.
> > 
> > > Source/WTF/wtf/gobject/GMutexLocker.h:42
> > > +        : m_mutex(0)
> > 
> > m_mutex(mutex)
> > 
> > > Source/WTF/wtf/gobject/GMutexLocker.h:62
> > > +    inline void lock(GMutex *mutex)
> > > +    {
> > > +        m_mutex = mutex;
> > > +        if (m_mutex)
> > > +            g_mutex_lock(m_mutex);
> > > +    }
> > > +
> > > +    inline void unlock()
> > > +    {
> > > +        if (m_mutex) {
> > > +            g_mutex_unlock(m_mutex);
> > > +            m_mutex = 0;
> > > +        }
> > > +    }
> > 
> > Looks like this should be private.
> > You have nothing preventing bad use with the current design:
> > 
> >     GMutexLocker foo;
> >     foo.lock(bar);
> >     foo.lock(baz);
> > 
> Tbh, I didn't want to create this class, I just did it cause Carlos requested on comment #28.
> 
> We could make the "lock" method private but we need a public "unlock" method at least as there are some cases we need to unlock it manually.
> 
> What do you suggest here? Should I make "lock" private? Should I revert this change and use GST_OBJECT_LOCK as it was before?
> 
> Note that having only a public "unlock" method would mean that once the GMutexLocker is unlocked it cannot be reused anymore, e.g.:
> 
>   {
>     GMutexLocker foo(bar);
>     doSomethingThatRequiresLock();
>     foo.unlock();
> 
>     doSomethingElse();
> 
>     // cannot lock bar again using "foo" locker, need to create a new one.
>     GMutexLocker foo1(bar);
>     doSomethingThatRequiresLock();
>   }
> 

Please check the attached GMutexLocker.h and let me know what you think and I will update the patch accordingly.

The new class design is more similar to QMutexLocker.h (see http://qt.gitorious.org/qt/qt/blobs/HEAD/src/corelib/thread/qmutex.h)

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.