Changeset 202615 in webkit


Ignore:
Timestamp:
Jun 28, 2016 11:33:57 PM (8 years ago)
Author:
Carlos Garcia Campos
Message:

[GStreamer] Adaptive streaming issues
https://bugs.webkit.org/show_bug.cgi?id=144040

Reviewed by Philippe Normand.

There are multiple deadlocks in the web process when HLS content is loaded by GStreamer. It happens because gst
is using several threads to download manifest, fragments, monitor the downloads, etc. To download the fragments
and manifest it always creates the source element in a separate thread, something that is not actually expected
to happen in WebKit source element. Our source element is always scheduling tasks (start, stop, need-data,
enough-data and seek) to the main thread, and those downloads that use the ResourceHandleStreamingClient
(there's no player associated) also happen in the main thread, because libsoup calls all its async callbacks in
the main thread. So, the result is that it can happen that we end up blocking the main thread in a lock until
the download finishes, but the download never finishes because tasks are scheduled in the main thread that is
blocked in a lock. This can be prevented by always using a secondary thread for downloads made by
ResourceHandleStreamingClient, using its own run loop with a different GMainContext so that libsoup sends
callbacks to the right thread. We also had to refactor the tasks a bit, leaving the thread safe parts to be run
in the calling thread always, and only scheduling to the main thread in case of not using
ResourceHandleStreamingClient and only for the non thread safe parts.
This patch also includes r200455 that was rolled out, but it was a perfectly valid workaround for GST bug.

  • platform/graphics/gstreamer/GRefPtrGStreamer.cpp:

(WTF::ensureGRef): Consume the floating ref if needed.

  • platform/graphics/gstreamer/GRefPtrGStreamer.h:
  • platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:

(webkit_web_src_init): Check if object is being created in the main thread.
(webKitWebSrcStop): Stop the media resource loader in the main thread and the resource handle streaming in the
current thread.
(webKitWebSrcStart): Start the media resource loader in the main thread and the resource handle streaming in
the current thread.
(webKitWebSrcChangeState): Call webKitWebSrcStart and webKitWebSrcStop in the current thread.
(webKitWebSrcNeedData): Update status in the current thread and notify the media resource loader in the main thread.
(webKitWebSrcEnoughData): Ditto.
(webKitWebSrcSeek): Ditto.
(webKitWebSrcSetMediaPlayer): Add an assert to ensure that source elements used by WebKit are always created in
the main thread.
(ResourceHandleStreamingClient::ResourceHandleStreamingClient): Use a secondary thread to do the download.
(ResourceHandleStreamingClient::~ResourceHandleStreamingClient): Stop the secondary thread.
(ResourceHandleStreamingClient::setDefersLoading): Notify the secondary thread.

Location:
trunk/Source/WebCore
Files:
4 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WebCore/ChangeLog

    r202614 r202615  
     12016-06-28  Carlos Garcia Campos  <cgarcia@igalia.com>
     2
     3        [GStreamer] Adaptive streaming issues
     4        https://bugs.webkit.org/show_bug.cgi?id=144040
     5
     6        Reviewed by Philippe Normand.
     7
     8        There are multiple deadlocks in the web process when HLS content is loaded by GStreamer. It happens because gst
     9        is using several threads to download manifest, fragments, monitor the downloads, etc. To download the fragments
     10        and manifest it always creates the source element in a separate thread, something that is not actually expected
     11        to happen in WebKit source element. Our source element is always scheduling tasks (start, stop, need-data,
     12        enough-data and seek) to the main thread, and those downloads that use the ResourceHandleStreamingClient
     13        (there's no player associated) also happen in the main thread, because libsoup calls all its async callbacks in
     14        the main thread. So, the result is that it can happen that we end up blocking the main thread in a lock until
     15        the download finishes, but the download never finishes because tasks are scheduled in the main thread that is
     16        blocked in a lock. This can be prevented by always using a secondary thread for downloads made by
     17        ResourceHandleStreamingClient, using its own run loop with a different GMainContext so that libsoup sends
     18        callbacks to the right thread. We also had to refactor the tasks a bit, leaving the thread safe parts to be run
     19        in the calling thread always, and only scheduling to the main thread in case of not using
     20        ResourceHandleStreamingClient and only for the non thread safe parts.
     21        This patch also includes r200455 that was rolled out, but it was a perfectly valid workaround for GST bug.
     22
     23        * platform/graphics/gstreamer/GRefPtrGStreamer.cpp:
     24        (WTF::ensureGRef): Consume the floating ref if needed.
     25        * platform/graphics/gstreamer/GRefPtrGStreamer.h:
     26        * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp:
     27        (webkit_web_src_init): Check if object is being created in the main thread.
     28        (webKitWebSrcStop): Stop the media resource loader in the main thread and the resource handle streaming in the
     29        current thread.
     30        (webKitWebSrcStart): Start the media resource loader in the main thread and the resource handle streaming in
     31        the current thread.
     32        (webKitWebSrcChangeState): Call webKitWebSrcStart and webKitWebSrcStop in the current thread.
     33        (webKitWebSrcNeedData): Update status in the current thread and notify the media resource loader in the main thread.
     34        (webKitWebSrcEnoughData): Ditto.
     35        (webKitWebSrcSeek): Ditto.
     36        (webKitWebSrcSetMediaPlayer): Add an assert to ensure that source elements used by WebKit are always created in
     37        the main thread.
     38        (ResourceHandleStreamingClient::ResourceHandleStreamingClient): Use a secondary thread to do the download.
     39        (ResourceHandleStreamingClient::~ResourceHandleStreamingClient): Stop the secondary thread.
     40        (ResourceHandleStreamingClient::setDefersLoading): Notify the secondary thread.
     41
    1422016-06-28  Youenn Fablet  <youennf@gmail.com>
    243
  • trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.cpp

    r202050 r202615  
    343343}
    344344
     345// This method is only available for WebKitWebSrc and should not be used for any other type.
     346// This is only to work around a bug in GST where the URI downloader is not taking the ownership of WebKitWebSrc.
     347// See https://bugs.webkit.org/show_bug.cgi?id=144040.
     348GRefPtr<WebKitWebSrc> ensureGRef(WebKitWebSrc* ptr)
     349{
     350    if (ptr && g_object_is_floating(ptr))
     351        gst_object_ref_sink(GST_OBJECT(ptr));
     352    return GRefPtr<WebKitWebSrc>(ptr);
     353}
     354
    345355template <> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr)
    346356{
  • trunk/Source/WebCore/platform/graphics/gstreamer/GRefPtrGStreamer.h

    r202050 r202615  
    109109
    110110template<> GRefPtr<WebKitWebSrc> adoptGRef(WebKitWebSrc* ptr);
     111GRefPtr<WebKitWebSrc> ensureGRef(WebKitWebSrc* ptr);
    111112template<> WebKitWebSrc* refGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
    112113template<> void derefGPtr<WebKitWebSrc>(WebKitWebSrc* ptr);
  • trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp

    r202556 r202615  
    8484    WTF_MAKE_NONCOPYABLE(ResourceHandleStreamingClient); WTF_MAKE_FAST_ALLOCATED;
    8585    public:
    86         ResourceHandleStreamingClient(WebKitWebSrc*, const ResourceRequest&);
     86        ResourceHandleStreamingClient(WebKitWebSrc*, ResourceRequest&&);
    8787        virtual ~ResourceHandleStreamingClient();
    8888
     
    105105        void cannotShowURL(ResourceHandle*) override;
    106106
     107        ThreadIdentifier m_thread { 0 };
     108        Lock m_initializeRunLoopConditionMutex;
     109        Condition m_initializeRunLoopCondition;
     110        RunLoop* m_runLoop { nullptr };
     111        Lock m_terminateRunLoopConditionMutex;
     112        Condition m_terminateRunLoopCondition;
    107113        RefPtr<ResourceHandle> m_resource;
    108114};
     
    130136    RefPtr<PlatformMediaResourceLoader> loader;
    131137    RefPtr<PlatformMediaResource> resource;
    132     ResourceHandleStreamingClient* client;
     138    std::unique_ptr<ResourceHandleStreamingClient> client;
    133139
    134140    bool didPassAccessControlCheck;
     
    137143    guint64 size;
    138144    gboolean seekable;
    139     gboolean paused;
     145    bool paused;
    140146    bool isSeeking;
    141147
    142148    guint64 requestedOffset;
    143149
     150    bool createdInMainThread;
    144151    MainThreadNotifier<MainThreadSourceNotification> notifier;
    145152    GRefPtr<GstBuffer> buffer;
     
    175182static void webKitWebSrcNeedData(WebKitWebSrc*);
    176183static void webKitWebSrcEnoughData(WebKitWebSrc*);
    177 static void webKitWebSrcSeek(WebKitWebSrc*);
     184static gboolean webKitWebSrcSeek(WebKitWebSrc*, guint64);
    178185
    179186static GstAppSrcCallbacks appsrcCallbacks = {
    180187    // need_data
    181188    [](GstAppSrc*, guint, gpointer userData) {
    182         WebKitWebSrc* src = WEBKIT_WEB_SRC(userData);
    183         WebKitWebSrcPrivate* priv = src->priv;
    184 
    185         {
    186             WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
    187             if (!priv->paused)
    188                 return;
    189         }
    190 
    191         GRefPtr<WebKitWebSrc> protector(src);
    192         priv->notifier.notify(MainThreadSourceNotification::NeedData, [protector] { webKitWebSrcNeedData(protector.get()); });
     189        webKitWebSrcNeedData(WEBKIT_WEB_SRC(userData));
    193190    },
    194191    // enough_data
    195192    [](GstAppSrc*, gpointer userData) {
    196         WebKitWebSrc* src = WEBKIT_WEB_SRC(userData);
    197         WebKitWebSrcPrivate* priv = src->priv;
    198 
    199         {
    200             WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
    201             if (priv->paused)
    202                 return;
    203         }
    204 
    205         GRefPtr<WebKitWebSrc> protector(src);
    206         priv->notifier.notify(MainThreadSourceNotification::EnoughData, [protector] { webKitWebSrcEnoughData(protector.get()); });
     193        webKitWebSrcEnoughData(WEBKIT_WEB_SRC(userData));
    207194    },
    208195    // seek_data
    209196    [](GstAppSrc*, guint64 offset, gpointer userData) -> gboolean {
    210         WebKitWebSrc* src = WEBKIT_WEB_SRC(userData);
    211         WebKitWebSrcPrivate* priv = src->priv;
    212 
    213         {
    214             WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
    215             if (offset == priv->offset && priv->requestedOffset == priv->offset)
    216                 return TRUE;
    217 
    218             if (!priv->seekable)
    219                 return FALSE;
    220 
    221             priv->isSeeking = true;
    222             priv->requestedOffset = offset;
    223         }
    224 
    225         GRefPtr<WebKitWebSrc> protector(src);
    226         priv->notifier.notify(MainThreadSourceNotification::Seek, [protector] { webKitWebSrcSeek(protector.get()); });
    227         return TRUE;
     197        return webKitWebSrcSeek(WEBKIT_WEB_SRC(userData), offset);
    228198    },
    229199    { nullptr }
     
    289259    src->priv = priv;
    290260    new (priv) WebKitWebSrcPrivate();
     261
     262    priv->createdInMainThread = isMainThread();
    291263
    292264    priv->appsrc = GST_APP_SRC(gst_element_factory_make("appsrc", 0));
     
    415387    WebKitWebSrcPrivate* priv = src->priv;
    416388
    417     ASSERT(isMainThread());
     389    if (priv->resource || (priv->loader && !priv->keepAlive)) {
     390        GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
     391        priv->notifier.cancelPendingNotifications(MainThreadSourceNotification::NeedData | MainThreadSourceNotification::EnoughData | MainThreadSourceNotification::Seek);
     392        priv->notifier.notify(MainThreadSourceNotification::Stop, [protector, keepAlive = priv->keepAlive] {
     393            WebKitWebSrcPrivate* priv = protector->priv;
     394
     395            WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(protector.get()));
     396            if (priv->resource) {
     397                priv->resource->stop();
     398                priv->resource->setClient(nullptr);
     399                priv->resource = nullptr;
     400            }
     401
     402            if (!keepAlive)
     403                priv->loader = nullptr;
     404        });
     405    }
    418406
    419407    WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
     
    421409    bool wasSeeking = std::exchange(priv->isSeeking, false);
    422410
    423     priv->notifier.cancelPendingNotifications(MainThreadSourceNotification::NeedData | MainThreadSourceNotification::EnoughData | MainThreadSourceNotification::Seek);
    424 
    425     if (priv->client) {
    426         delete priv->client;
    427         priv->client = 0;
    428     }
    429 
    430     if (!priv->keepAlive)
    431         priv->loader = nullptr;
     411    priv->client = nullptr;
    432412
    433413    if (priv->buffer) {
     
    436416    }
    437417
    438     priv->paused = FALSE;
     418    priv->paused = false;
    439419
    440420    priv->offset = 0;
     
    512492{
    513493    WebKitWebSrcPrivate* priv = src->priv;
    514 
    515     ASSERT(isMainThread());
    516494
    517495    WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
     
    579557    request.setHTTPHeaderField(HTTPHeaderName::IcyMetadata, "1");
    580558
    581     bool loadFailed = true;
    582     if (priv->player && !priv->loader)
    583         priv->loader = priv->player->createResourceLoader();
    584 
    585     if (priv->loader) {
     559    if (!priv->player || !priv->createdInMainThread) {
     560        priv->client = std::make_unique<ResourceHandleStreamingClient>(src, WTFMove(request));
     561        if (priv->client->loadFailed()) {
     562            GST_ERROR_OBJECT(src, "Failed to setup streaming client");
     563            priv->client = nullptr;
     564            locker.unlock();
     565            webKitWebSrcStop(src);
     566        } else
     567            GST_DEBUG_OBJECT(src, "Started request");
     568        return;
     569    }
     570
     571    locker.unlock();
     572    GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
     573    priv->notifier.notify(MainThreadSourceNotification::Start, [protector, request = WTFMove(request)] {
     574        WebKitWebSrcPrivate* priv = protector->priv;
     575
     576        WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(protector.get()));
     577        if (!priv->loader)
     578            priv->loader = priv->player->createResourceLoader();
     579
    586580        PlatformMediaResourceLoader::LoadOptions loadOptions = 0;
    587581        if (request.url().protocolIsBlob())
    588582            loadOptions |= PlatformMediaResourceLoader::LoadOption::BufferData;
    589583        priv->resource = priv->loader->requestResource(request, loadOptions);
    590         loadFailed = !priv->resource;
    591 
    592         if (priv->resource)
    593             priv->resource->setClient(std::make_unique<CachedResourceStreamingClient>(src));
    594     } else {
    595         priv->client = new ResourceHandleStreamingClient(src, request);
    596         loadFailed = priv->client->loadFailed();
    597     }
    598 
    599     if (loadFailed) {
    600         GST_ERROR_OBJECT(src, "Failed to setup streaming client");
    601         if (priv->client) {
    602             delete priv->client;
    603             priv->client = nullptr;
     584        if (priv->resource) {
     585            priv->resource->setClient(std::make_unique<CachedResourceStreamingClient>(protector.get()));
     586            GST_DEBUG_OBJECT(protector.get(), "Started request");
     587        } else {
     588            GST_ERROR_OBJECT(protector.get(), "Failed to setup streaming client");
     589            priv->loader = nullptr;
     590            locker.unlock();
     591            webKitWebSrcStop(protector.get());
    604592        }
    605         priv->loader = nullptr;
    606         priv->resource = nullptr;
    607         locker.unlock();
    608         webKitWebSrcStop(src);
    609         return;
    610     }
    611     GST_DEBUG_OBJECT(src, "Started request");
     593    });
    612594}
    613595
     
    641623    {
    642624        GST_DEBUG_OBJECT(src, "READY->PAUSED");
    643         GRefPtr<WebKitWebSrc> protector(src);
    644         priv->notifier.notify(MainThreadSourceNotification::Start, [protector] { webKitWebSrcStart(protector.get()); });
     625        webKitWebSrcStart(src);
    645626        break;
    646627    }
     
    648629    {
    649630        GST_DEBUG_OBJECT(src, "PAUSED->READY");
    650         priv->notifier.cancelPendingNotifications();
    651         GRefPtr<WebKitWebSrc> protector(src);
    652         priv->notifier.notify(MainThreadSourceNotification::Stop, [protector] { webKitWebSrcStop(protector.get()); });
     631        webKitWebSrcStop(src);
    653632        break;
    654633    }
     
    782761}
    783762
    784 // appsrc callbacks
    785 
    786763static void webKitWebSrcNeedData(WebKitWebSrc* src)
    787764{
    788765    WebKitWebSrcPrivate* priv = src->priv;
    789 
    790     ASSERT(isMainThread());
    791766
    792767    GST_DEBUG_OBJECT(src, "Need more data");
     
    794769    {
    795770        WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
    796         priv->paused = FALSE;
    797     }
    798 
    799     if (priv->client)
    800         priv->client->setDefersLoading(false);
    801     if (priv->resource)
    802         priv->resource->setDefersLoading(false);
     771        if (!priv->paused)
     772            return;
     773        priv->paused = false;
     774        if (priv->client) {
     775            priv->client->setDefersLoading(false);
     776            return;
     777        }
     778    }
     779
     780    GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
     781    priv->notifier.notify(MainThreadSourceNotification::NeedData, [protector] {
     782        WebKitWebSrcPrivate* priv = protector->priv;
     783        if (priv->resource)
     784            priv->resource->setDefersLoading(false);
     785    });
    803786}
    804787
     
    806789{
    807790    WebKitWebSrcPrivate* priv = src->priv;
    808 
    809     ASSERT(isMainThread());
    810791
    811792    GST_DEBUG_OBJECT(src, "Have enough data");
     
    813794    {
    814795        WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
    815         priv->paused = TRUE;
    816     }
    817 
    818     if (priv->client)
    819         priv->client->setDefersLoading(true);
    820     if (priv->resource)
    821         priv->resource->setDefersLoading(true);
    822 }
    823 
    824 static void webKitWebSrcSeek(WebKitWebSrc* src)
    825 {
    826     ASSERT(isMainThread());
     796        if (priv->paused)
     797            return;
     798        priv->paused = true;
     799        if (priv->client) {
     800            priv->client->setDefersLoading(true);
     801            return;
     802        }
     803    }
     804
     805    GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
     806    priv->notifier.notify(MainThreadSourceNotification::EnoughData, [protector] {
     807        WebKitWebSrcPrivate* priv = protector->priv;
     808        if (priv->resource)
     809            priv->resource->setDefersLoading(true);
     810    });
     811}
     812
     813static gboolean webKitWebSrcSeek(WebKitWebSrc* src, guint64 offset)
     814{
     815    WebKitWebSrcPrivate* priv = src->priv;
     816
     817    {
     818        WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
     819        if (offset == priv->offset && priv->requestedOffset == priv->offset)
     820            return TRUE;
     821
     822        if (!priv->seekable)
     823            return FALSE;
     824
     825        priv->isSeeking = true;
     826        priv->requestedOffset = offset;
     827    }
    827828
    828829    GST_DEBUG_OBJECT(src, "Seeking to offset: %" G_GUINT64_FORMAT, src->priv->requestedOffset);
    829 
    830     webKitWebSrcStop(src);
    831     webKitWebSrcStart(src);
     830    if (priv->client) {
     831        webKitWebSrcStop(src);
     832        webKitWebSrcStart(src);
     833        return TRUE;
     834    }
     835
     836    GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src);
     837    priv->notifier.notify(MainThreadSourceNotification::Seek, [protector] {
     838        webKitWebSrcStop(protector.get());
     839        webKitWebSrcStart(protector.get());
     840    });
     841    return TRUE;
    832842}
    833843
     
    835845{
    836846    ASSERT(player);
     847    ASSERT(src->priv->createdInMainThread);
    837848    WTF::GMutexLocker<GMutex> locker(*GST_OBJECT_GET_LOCK(src));
    838849    src->priv->player = player;
     
    10611072}
    10621073
    1063 ResourceHandleStreamingClient::ResourceHandleStreamingClient(WebKitWebSrc* src, const ResourceRequest& request)
     1074ResourceHandleStreamingClient::ResourceHandleStreamingClient(WebKitWebSrc* src, ResourceRequest&& request)
    10641075    : StreamingClient(src)
    10651076{
    1066     m_resource = ResourceHandle::create(0 /*context*/, request, this, false, false);
     1077    LockHolder locker(m_initializeRunLoopConditionMutex);
     1078    m_thread = createThread("ResourceHandleStreamingClient", [this, request = WTFMove(request)] {
     1079        {
     1080            LockHolder locker(m_initializeRunLoopConditionMutex);
     1081            m_runLoop = &RunLoop::current();
     1082            m_resource = ResourceHandle::create(nullptr /*context*/, request, this, true, false);
     1083            m_initializeRunLoopCondition.notifyOne();
     1084        }
     1085        if (!m_resource)
     1086            return;
     1087
     1088        m_runLoop->dispatch([this] { m_resource->setDefersLoading(false); });
     1089        m_runLoop->run();
     1090        {
     1091            LockHolder locker(m_terminateRunLoopConditionMutex);
     1092            m_runLoop = nullptr;
     1093            m_resource->clearClient();
     1094            m_resource->cancel();
     1095            m_resource = nullptr;
     1096            m_terminateRunLoopCondition.notifyOne();
     1097        }
     1098    });
     1099    m_initializeRunLoopCondition.wait(m_initializeRunLoopConditionMutex);
    10671100}
    10681101
    10691102ResourceHandleStreamingClient::~ResourceHandleStreamingClient()
    10701103{
    1071     if (m_resource) {
    1072         m_resource->cancel();
    1073         m_resource = nullptr;
     1104    if (m_thread) {
     1105        detachThread(m_thread);
     1106        m_thread = 0;
     1107    }
     1108
     1109    if (m_runLoop == &RunLoop::current())
     1110        m_runLoop->stop();
     1111    else {
     1112        LockHolder locker(m_terminateRunLoopConditionMutex);
     1113        m_runLoop->stop();
     1114        m_terminateRunLoopCondition.wait(m_terminateRunLoopConditionMutex);
    10741115    }
    10751116}
     
    10821123void ResourceHandleStreamingClient::setDefersLoading(bool defers)
    10831124{
    1084     if (m_resource)
    1085         m_resource->setDefersLoading(defers);
     1125    m_runLoop->dispatch([this, defers] {
     1126        if (m_resource)
     1127            m_resource->setDefersLoading(defers);
     1128    });
    10861129}
    10871130
Note: See TracChangeset for help on using the changeset viewer.