[webkit-reviews] review canceled: [Bug 115352] [gstreamer] Make sure gstreamer source element is thread-safe : [Attachment 209473] Patch

[bugzilla-daemon at webkit.org]

Andre Moreira Magalhaes <andrunko at gmail.com> has canceled Andre Moreira
Magalhaes <andrunko at gmail.com>'s request for review:
Bug 115352: [gstreamer] Make sure gstreamer source element is thread-safe
https://bugs.webkit.org/show_bug.cgi?id=115352

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

------- Additional Comments from Andre Moreira Magalhaes <andrunko at gmail.com>
(In reply to comment #40)
> (From update of attachment 209473 [details])
> View in context:
https://bugs.webkit.org/attachment.cgi?id=209473&action=review
> 
> > Source/WTF/wtf/gobject/GMutexLocker.h:36
> > +	 inline explicit GMutexLocker(GMutex *mutex)
> 
> Misplaced * :)
>
Arg, fixed, strange that check-style didn't catch it :/.
  
> > Source/WTF/wtf/gobject/GMutexLocker.h:48
> > +		 g_mutex_lock(m_mutex);
> 
> I wonder if instead of _lock here we could _try_lock and ASSERT on failure,
would it make sense?
hmm, I don't think we want that actually. "trylock" would just return FALSE if
the mutex is already locked by another thread instead of blocking. Quoting from
gmutex docs:

  void g_mutex_lock (GMutex *mutex);
  Locks mutex. If mutex is already locked by another thread, the current thread
will block until mutex is unlocked by the other thread.
  
  gboolean g_mutex_trylock (GMutex *mutex);
  Tries to lock mutex. If mutex is already locked by another thread, it
immediately returns FALSE. Otherwise it locks mutex and returns TRUE.