[webkit-reviews] review denied: [Bug 195453] [GStreamer][v4l2] Synchronous video texture flushing support : [Attachment 364367] Patch

[bugzilla-daemon at webkit.org]

Xabier Rodríguez Calvar <calvaris at igalia.com> has denied Philippe Normand
<pnormand at igalia.com>'s request for review:
Bug 195453: [GStreamer][v4l2] Synchronous video texture flushing support
https://bugs.webkit.org/show_bug.cgi?id=195453

Attachment 364367: Patch

https://bugs.webkit.org/attachment.cgi?id=364367&action=review




--- Comment #15 from Xabier Rodríguez Calvar <calvaris at igalia.com> ---
Comment on attachment 364367
  --> https://bugs.webkit.org/attachment.cgi?id=364367
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=364367&action=review

>
Source/WebCore/platform/graphics/gstreamer/MediaPlayerPrivateGStreamerBase.cpp:
868
> +	       if (!shouldWait)
> +		   proxy.lock().lock();

I think there's a problem here when I told you to remove the LockHolder and do
a manual lock, that we materialized one of the problems of not using a
LockHolder, that is that the lock might be held forever. It looks like know
you're holding the lock and never releasing it.

Let's turn again to a LockHolder but let's do it like:
LockHolder locker(!shouldWait ? proxy.lock() : nullptr);

This way the code is still not repeated and we hold the lock when we should,
releasing it when we should as well.

Am I analizing it wrong?