- Revision
- 278655
- Author
- [email protected]
- Date
- 2021-06-09 03:58:36 -0700 (Wed, 09 Jun 2021)
Log Message
[WTF][GStreamer] Add RAII lockers for 3rd party locks
https://bugs.webkit.org/show_bug.cgi?id=225650
Reviewed by Xabier Rodriguez-Calvar.
Source/WebCore:
This patch introduces RAII locker classes that wrap GST_OBJECT_LOCK
and GST_PAD_STREAM_LOCK to match the style, safety and convenience of
locks from WTF.
This patch also changes all usages of GStreamer locks in the WebKit
codebase to use these new lockers.
This patch introduces no behavior changes.
* platform/graphics/gstreamer/GStreamerCommon.h:
(gstObjectLock):
(gstObjectUnlock):
(gstPadStreamLock):
(gstPadStreamUnlock):
(holdGstObjectLock):
(holdGstPadStreamLock):
* platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp:
(webkitTextCombinerPadGetProperty):
(webkitTextCombinerPadSetProperty):
* platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:
(webKitMediaSrcWaitForPadLinkedOrFlush):
(webKitMediaSrcLoop):
(webKitMediaSrcStreamFlush):
(webKitMediaSrcGetUri):
(webKitMediaSrcSetUri):
Source/WTF:
This patch introduces WTF::ExternalLocker, which allows to lock 3rd
party mutexes in a RAII fashion, very similar to WTF::Locker.
This is used also in WebCore to provide RAII lockers for GStreamer.
* wtf/Locker.h:
(WTF::unlockFunction):
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (278654 => 278655)
--- trunk/Source/WTF/ChangeLog 2021-06-09 08:35:40 UTC (rev 278654)
+++ trunk/Source/WTF/ChangeLog 2021-06-09 10:58:36 UTC (rev 278655)
@@ -1,3 +1,18 @@
+2021-06-09 Alicia Boya García <[email protected]>
+
+ [WTF][GStreamer] Add RAII lockers for 3rd party locks
+ https://bugs.webkit.org/show_bug.cgi?id=225650
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ This patch introduces WTF::ExternalLocker, which allows to lock 3rd
+ party mutexes in a RAII fashion, very similar to WTF::Locker.
+
+ This is used also in WebCore to provide RAII lockers for GStreamer.
+
+ * wtf/Locker.h:
+ (WTF::unlockFunction):
+
2021-06-08 Alex Christensen <[email protected]>
Move PrivacyStance code from WebKitAdditions
Modified: trunk/Source/WTF/wtf/Locker.h (278654 => 278655)
--- trunk/Source/WTF/wtf/Locker.h 2021-06-09 08:35:40 UTC (rev 278654)
+++ trunk/Source/WTF/wtf/Locker.h 2021-06-09 10:58:36 UTC (rev 278655)
@@ -160,6 +160,66 @@
Locker<LockType>& m_lock;
};
+// This is a close replica of Locker, but for generic lock/unlock functions.
+template<typename T, void (lockFunction)(T*), void (*unlockFunction)(T*)>
+class ExternalLocker: public WTF::AbstractLocker {
+public:
+ explicit ExternalLocker(T* lockable)
+ : m_lockable(lockable)
+ {
+ ASSERT(lockable);
+ lock();
+ }
+
+ ~ExternalLocker()
+ {
+ unlock();
+ }
+
+ T* lockable() { return m_lockable; }
+
+ explicit operator bool() const { return !!m_lockable; }
+
+ void unlockEarly()
+ {
+ unlock();
+ m_lockable = nullptr;
+ }
+
+ ExternalLocker(ExternalLocker&& other)
+ : m_lockable(other.m_lockable)
+ {
+ ASSERT(&other != this);
+ other.m_lockable = nullptr;
+ }
+
+ ExternalLocker& operator=(ExternalLocker&& other)
+ {
+ ASSERT(&other != this);
+ m_lockable = other.m_lockable;
+ other.m_lockable = nullptr;
+ return *this;
+ }
+
+private:
+ template<typename>
+ friend class DropLockForScope;
+
+ void unlock()
+ {
+ if (m_lockable)
+ unlockFunction(m_lockable);
+ }
+
+ void lock()
+ {
+ if (m_lockable)
+ lockFunction(m_lockable);
+ }
+
+ T* m_lockable;
+};
+
}
using WTF::AbstractLocker;
@@ -168,3 +228,4 @@
using WTF::NoLockingNecessaryTag;
using WTF::NoLockingNecessary;
using WTF::DropLockForScope;
+using WTF::ExternalLocker;
Modified: trunk/Source/WebCore/ChangeLog (278654 => 278655)
--- trunk/Source/WebCore/ChangeLog 2021-06-09 08:35:40 UTC (rev 278654)
+++ trunk/Source/WebCore/ChangeLog 2021-06-09 10:58:36 UTC (rev 278655)
@@ -1,3 +1,36 @@
+2021-06-09 Alicia Boya García <[email protected]>
+
+ [WTF][GStreamer] Add RAII lockers for 3rd party locks
+ https://bugs.webkit.org/show_bug.cgi?id=225650
+
+ Reviewed by Xabier Rodriguez-Calvar.
+
+ This patch introduces RAII locker classes that wrap GST_OBJECT_LOCK
+ and GST_PAD_STREAM_LOCK to match the style, safety and convenience of
+ locks from WTF.
+
+ This patch also changes all usages of GStreamer locks in the WebKit
+ codebase to use these new lockers.
+
+ This patch introduces no behavior changes.
+
+ * platform/graphics/gstreamer/GStreamerCommon.h:
+ (gstObjectLock):
+ (gstObjectUnlock):
+ (gstPadStreamLock):
+ (gstPadStreamUnlock):
+ (holdGstObjectLock):
+ (holdGstPadStreamLock):
+ * platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp:
+ (webkitTextCombinerPadGetProperty):
+ (webkitTextCombinerPadSetProperty):
+ * platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp:
+ (webKitMediaSrcWaitForPadLinkedOrFlush):
+ (webKitMediaSrcLoop):
+ (webKitMediaSrcStreamFlush):
+ (webKitMediaSrcGetUri):
+ (webKitMediaSrcSetUri):
+
2021-06-09 Chris Dumez <[email protected]>
Rely on SQLiteDatabase::setMaximumSize() for quota management in LocalStorageDatabase
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h (278654 => 278655)
--- trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h 2021-06-09 08:35:40 UTC (rev 278654)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/GStreamerCommon.h 2021-06-09 10:58:36 UTC (rev 278655)
@@ -320,4 +320,13 @@
#define webkitGstAudioFormatFillSilence gst_audio_format_fill_silence
#endif
+// We can't pass macros as template parameters, so we need to wrap them in inline functions.
+inline void gstObjectLock(void* object) { GST_OBJECT_LOCK(object); }
+inline void gstObjectUnlock(void* object) { GST_OBJECT_UNLOCK(object); }
+inline void gstPadStreamLock(GstPad* pad) { GST_PAD_STREAM_LOCK(pad); }
+inline void gstPadStreamUnlock(GstPad* pad) { GST_PAD_STREAM_UNLOCK(pad); }
+
+using GstObjectLocker = ExternalLocker<void, gstObjectLock, gstObjectUnlock>;
+using GstPadStreamLocker = ExternalLocker<GstPad, gstPadStreamLock, gstPadStreamUnlock>;
+
#endif // USE(GSTREAMER)
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp (278654 => 278655)
--- trunk/Source/WebCore/platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp 2021-06-09 08:35:40 UTC (rev 278654)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/TextCombinerPadGStreamer.cpp 2021-06-09 10:58:36 UTC (rev 278655)
@@ -62,12 +62,13 @@
gst_event_parse_tag(event, &tags);
ASSERT(tags);
- GST_OBJECT_LOCK(pad);
- if (!combinerPad->priv->tags)
- combinerPad->priv->tags = adoptGRef(gst_tag_list_copy(tags));
- else
- gst_tag_list_insert(combinerPad->priv->tags.get(), tags, GST_TAG_MERGE_REPLACE);
- GST_OBJECT_UNLOCK(pad);
+ {
+ auto locker = GstObjectLocker(pad);
+ if (!combinerPad->priv->tags)
+ combinerPad->priv->tags = adoptGRef(gst_tag_list_copy(tags));
+ else
+ gst_tag_list_insert(combinerPad->priv->tags.get(), tags, GST_TAG_MERGE_REPLACE);
+ }
g_object_notify_by_pspec(G_OBJECT(pad), sObjProperties[PROP_PAD_TAGS]);
break;
@@ -82,17 +83,17 @@
{
auto* pad = WEBKIT_TEXT_COMBINER_PAD(object);
switch (propertyId) {
- case PROP_PAD_TAGS:
- GST_OBJECT_LOCK(object);
+ case PROP_PAD_TAGS: {
+ auto locker = GstObjectLocker(object);
if (pad->priv->tags)
g_value_take_boxed(value, gst_tag_list_copy(pad->priv->tags.get()));
- GST_OBJECT_UNLOCK(object);
break;
- case PROP_INNER_COMBINER_PAD:
- GST_OBJECT_LOCK(object);
+ }
+ case PROP_INNER_COMBINER_PAD: {
+ auto locker = GstObjectLocker(object);
g_value_set_object(value, pad->priv->innerCombinerPad.get());
- GST_OBJECT_UNLOCK(object);
break;
+ }
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID(object, propertyId, pspec);
break;
@@ -103,11 +104,11 @@
{
auto* pad = WEBKIT_TEXT_COMBINER_PAD(object);
switch (propertyId) {
- case PROP_INNER_COMBINER_PAD:
- GST_OBJECT_LOCK(object);
+ case PROP_INNER_COMBINER_PAD: {
+ auto locker = GstObjectLocker(object);
pad->priv->innerCombinerPad = adoptGRef(GST_PAD_CAST(g_value_get_object(value)));
- GST_OBJECT_UNLOCK(object);
break;
+ }
default:
G_OBJECT_WARN_INVALID_PROPERTY_ID(object, propertyId, pspec);
break;
Modified: trunk/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp (278654 => 278655)
--- trunk/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp 2021-06-09 08:35:40 UTC (rev 278654)
+++ trunk/Source/WebCore/platform/graphics/gstreamer/mse/WebKitMediaSourceGStreamer.cpp 2021-06-09 10:58:36 UTC (rev 278655)
@@ -368,6 +368,24 @@
streamingMembers->padLinkedOrFlushedCondition.notifyOne();
}
+static void webKitMediaSrcWaitForPadLinkedOrFlush(GstPad* pad, DataMutexLocker<Stream::StreamingMembers>& streamingMembers)
+{
+ {
+ auto locker = GstObjectLocker(pad);
+ if (LIKELY(GST_PAD_IS_LINKED(pad)))
+ return;
+
+ GST_DEBUG_OBJECT(pad, "Waiting for the pad to be linked...");
+ g_signal_connect(pad, "linked", G_CALLBACK(webKitMediaSrcPadLinked), nullptr);
+ }
+
+ assertIsHeld(streamingMembers.mutex());
+ streamingMembers->padLinkedOrFlushedCondition.wait(streamingMembers.mutex());
+
+ g_signal_handlers_disconnect_by_func(pad, reinterpret_cast<void*>(webKitMediaSrcPadLinked), nullptr);
+ GST_DEBUG_OBJECT(pad, "Finished waiting for the pad to be linked.");
+}
+
// Called with STREAM_LOCK.
static void webKitMediaSrcLoop(void* userData)
{
@@ -382,22 +400,11 @@
// Since the pad can and will be added when the element is in PLAYING state, this task can start running
// before the pad is linked. Wait for the pad to be linked to avoid buffers being lost to not-linked errors.
- GST_OBJECT_LOCK(pad);
- if (!GST_PAD_IS_LINKED(pad)) {
- GST_DEBUG_OBJECT(pad, "Waiting for the pad to be linked...");
- g_signal_connect(pad, "linked", G_CALLBACK(webKitMediaSrcPadLinked), nullptr);
- GST_OBJECT_UNLOCK(pad);
-
- streamingMembers->padLinkedOrFlushedCondition.wait(streamingMembers.mutex());
-
- g_signal_handlers_disconnect_by_func(pad, reinterpret_cast<void*>(webKitMediaSrcPadLinked), nullptr);
- GST_DEBUG_OBJECT(pad, "Finished waiting for the pad to be linked.");
- if (streamingMembers->isFlushing) {
- gst_pad_pause_task(pad);
- return;
- }
- } else
- GST_OBJECT_UNLOCK(pad);
+ webKitMediaSrcWaitForPadLinkedOrFlush(pad, streamingMembers);
+ if (streamingMembers->isFlushing) {
+ gst_pad_pause_task(pad);
+ return;
+ }
ASSERT(gst_pad_is_linked(pad));
// By keeping the lock we are guaranteed that a flush will not happen while we send essential events.
@@ -611,7 +618,7 @@
if (!skipFlush) {
// By taking the stream lock we are waiting for the streaming thread task to stop if it hadn't yet.
GST_DEBUG_OBJECT(stream->pad.get(), "Taking the STREAM_LOCK.");
- GST_PAD_STREAM_LOCK(stream->pad.get());
+ auto streamLock = GstPadStreamLocker(stream->pad.get());
{
GST_DEBUG_OBJECT(stream->pad.get(), "Taking the StreamingMembers mutex again.");
DataMutexLocker streamingMembers { stream->streamingMembersDataMutex };
@@ -627,7 +634,6 @@
GST_DEBUG_OBJECT(stream->pad.get(), "Starting webKitMediaSrcLoop task and releasing the STREAM_LOCK.");
gst_pad_start_task(stream->pad.get(), webKitMediaSrcLoop, stream->pad.get(), nullptr);
- GST_PAD_STREAM_UNLOCK(stream->pad.get());
}
}
@@ -737,12 +743,9 @@
static gchar* webKitMediaSrcGetUri(GstURIHandler* handler)
{
WebKitMediaSrc* source = WEBKIT_MEDIA_SRC(handler);
- gchar* result;
- GST_OBJECT_LOCK(source);
- result = g_strdup(source->priv->uri.get());
- GST_OBJECT_UNLOCK(source);
- return result;
+ auto locker = GstObjectLocker(source);
+ return g_strdup(source->priv->uri.get());
}
static gboolean webKitMediaSrcSetUri(GstURIHandler* handler, const gchar* uri, GError**)
@@ -754,9 +757,8 @@
return false;
}
- GST_OBJECT_LOCK(source);
+ auto locker = GstObjectLocker(source);
source->priv->uri = GUniquePtr<char>(g_strdup(uri));
- GST_OBJECT_UNLOCK(source);
return TRUE;
}
