Changeset 247215 in webkit
- Timestamp:
- Jul 8, 2019 11:16:08 AM (5 years ago)
- Location:
- trunk
- Files:
-
- 1 added
- 4 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/Source/WebCore/ChangeLog
r247213 r247215 1 2019-07-08 Charlie Turner <cturner@igalia.com> 2 3 REGRESSION(r243197): [GStreamer] Web process hangs when scrolling twitter timeline which contains HLS videos 4 https://bugs.webkit.org/show_bug.cgi?id=197558 5 6 Reviewed by Xabier Rodriguez-Calvar. 7 8 Not covered, I have a test locally that would probably trigger the 9 deadlock if the network requests took a realistic amount of time, 10 but from a local webserver the window of time to hit this deadlock 11 is too narrow. 12 13 * platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp: 14 (webkit_web_src_init): Make the websrc start asynchronously, this 15 allows the main thread to be free to complete resource loader 16 setup. 17 (webKitWebSrcCreate): Calling start() from the create() vfunc is a 18 recipe for deadlock, since BaseSrc holds the streaming lock during 19 seeks, and then calls create(). In these cases, we do not want to 20 notify async-completion, since we've already completed from the 21 necessarily preceeding start() vfunc, and calling it again would 22 require the stream-lock and deadlock us. 23 (webKitWebSrcStart): Refactor to use webKitWebSrcMakeRequest, but 24 ensuring that we do perform an async-complete notification. 25 (webKitWebSrcMakeRequest): What Start() used to be, but now can be 26 toggled when to notify of async-completion. Start() no longer 27 blocks, since the return value of initiating a resource loader is 28 of no interest to the callers. 29 (webKitWebSrcCloseSession): Similarly to Start(), we do not need 30 to wait for the completion of cancelled net requests. 31 1 32 2019-07-08 Chris Dumez <cdumez@apple.com> 2 33 -
trunk/Source/WebCore/platform/graphics/gstreamer/WebKitWebSourceGStreamer.cpp
r247010 r247215 157 157 static GstStateChangeReturn webKitWebSrcChangeState(GstElement*, GstStateChange); 158 158 static GstFlowReturn webKitWebSrcCreate(GstPushSrc*, GstBuffer**); 159 static gboolean webKitWebSrcMakeRequest(GstBaseSrc*, bool); 159 160 static gboolean webKitWebSrcStart(GstBaseSrc*); 160 161 static gboolean webKitWebSrcStop(GstBaseSrc*); … … 261 262 webkitWebSrcReset(src); 262 263 gst_base_src_set_automatic_eos(GST_BASE_SRC_CAST(src), FALSE); 264 gst_base_src_set_async(GST_BASE_SRC_CAST(src), TRUE); 263 265 } 264 266 … … 362 364 webKitWebSrcStop(baseSrc); 363 365 priv->requestedPosition = requestedPosition; 364 webKitWebSrcStart(baseSrc); 366 // Do not notify async-completion, in seeking flows, we will 367 // be called from GstBaseSrc's perform_seek vfunc, which holds 368 // a streaming lock in our frame. Hence, we would deadlock 369 // trying to notify async completion, since that also requires 370 // the streaming lock. 371 webKitWebSrcMakeRequest(baseSrc, false); 365 372 } 366 373 … … 498 505 static gboolean webKitWebSrcStart(GstBaseSrc* baseSrc) 499 506 { 507 // This method should only be called by BaseSrc, do not call it 508 // from ourselves unless you ensure the streaming lock is not 509 // held. If it is, you will deadlock the WebProcess. 510 return webKitWebSrcMakeRequest(baseSrc, true); 511 } 512 513 static gboolean webKitWebSrcMakeRequest(GstBaseSrc* baseSrc, bool notifyAsyncCompletion) 514 { 500 515 WebKitWebSrc* src = WEBKIT_WEB_SRC(baseSrc); 501 516 WebKitWebSrcPrivate* priv = src->priv; … … 583 598 584 599 GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src); 585 priv->notifier->notify AndWait(MainThreadSourceNotification::Start, [protector, request = WTFMove(request)] {600 priv->notifier->notify(MainThreadSourceNotification::Start, [protector, request = WTFMove(request), src, notifyAsyncCompletion] { 586 601 WebKitWebSrcPrivate* priv = protector->priv; 587 602 if (!priv->loader) … … 595 610 priv->resource->setClient(std::make_unique<CachedResourceStreamingClient>(protector.get(), ResourceRequest(request))); 596 611 GST_DEBUG_OBJECT(protector.get(), "Started request"); 612 if (notifyAsyncCompletion) 613 gst_base_src_start_complete(GST_BASE_SRC(src), GST_FLOW_OK); 597 614 } else { 598 615 GST_ERROR_OBJECT(protector.get(), "Failed to setup streaming client"); 616 if (notifyAsyncCompletion) 617 gst_base_src_start_complete(GST_BASE_SRC(src), GST_FLOW_ERROR); 599 618 priv->loader = nullptr; 600 619 } 601 620 }); 602 621 603 GST_DEBUG_OBJECT(src, "Resource loader started");604 622 return TRUE; 605 623 } … … 610 628 GRefPtr<WebKitWebSrc> protector = WTF::ensureGRef(src); 611 629 612 priv->notifier->notify AndWait(MainThreadSourceNotification::Stop, [protector, keepAlive = priv->keepAlive] {630 priv->notifier->notify(MainThreadSourceNotification::Stop, [protector, keepAlive = priv->keepAlive] { 613 631 WebKitWebSrcPrivate* priv = protector->priv; 614 632 -
trunk/Tools/ChangeLog
r247205 r247215 1 2019-07-08 Charlie Turner <cturner@igalia.com> 2 3 REGRESSION(r243197): [GStreamer] Web process hangs when scrolling twitter timeline which contains HLS videos 4 https://bugs.webkit.org/show_bug.cgi?id=197558 5 6 Reviewed by Xabier Rodriguez-Calvar. 7 8 On shutdown we can easily deadlock the web process if we don't 9 ensure all network operations are completed before comitting state 10 changes. In HLS, make sure the network operations are cancelled, 11 and also prevent hlsdemux's retry logic from scuppering our 12 efforts. 13 14 * gstreamer/jhbuild.modules: Include the patch. 15 * gstreamer/patches/gst-plugins-bad-do-not-retry-downloads-during-shutdown.patch: Added. 16 1 17 2019-07-08 Antoine Quint <graouts@apple.com> 2 18 -
trunk/Tools/gstreamer/jhbuild.modules
r245677 r247215 93 93 </dependencies> 94 94 <branch hash="sha256:22139de35626ada6090bdfa3423b27b7fc15a0198331d25c95e6b12cb1072b05" module="gst-plugins-bad/gst-plugins-bad-${version}.tar.xz" repo="gstreamer" version="1.16.0"> 95 <patch file="gst-plugins-bad-do-not-retry-downloads-during-shutdown.patch" strip="1"/> <!-- In review: https://gitlab.freedesktop.org/gstreamer/gst-plugins-bad/merge_requests/427 --> 95 96 </branch> 96 97 </meson>
Note: See TracChangeset
for help on using the changeset viewer.