WebPlatformForEmbedded / WPEWebKit

WPE WebKit port (downstream)
213 stars 136 forks source link

WebProcess hangs while playing long audio #774

Closed varumugam123 closed 2 years ago

varumugam123 commented 2 years ago

Crashes are observed while playing long audio using WebkitWebSrc. The crash was observed on both MIPS & ARM devices and with ARM devices it takes few more attempts to reproduce the issue.

The crash is observed on many apps mostly iheart, pandora. But could be reproduced with the below simple page as well

<!DOCTYPE html>
<html lang="en">
  <head>
    <title>Stream player</title>
    <meta charset="UTF-8" />
    <link rel="shortcut icon" href="#" />
  </head>
  <body style="margin: 0px;">
    <audio controls src="https://traffic.libsyn.com/secure/draudioarchives/10122021_the_dave_ramsey_show_archive_3.mp3?dest-id=602044&source=iheart" autoplay=true>
    </audio>
  </body>
</html>

Attaching stack trace from an instance: iheart-hang-crash-01.txt

It looks like, if queue2 has large buffers / more smaller buffers which can play for more than 30 seconds, main threads gets hung for that entire period if it attempts to send custom event with "http-headers".

PS: 30 seconds is the maximum time set from UI process for detecting unresponsive webprocess, beyond which webprocess would be killed. The above mentioned crash is a case of that i.e main thread of webprocess waits for more than 30 seconds and doesn't respond to ping messages from ui process and eventually gets killed.

varumugam123 commented 2 years ago

This happened with wpe-2.22 : a79319e92c

varumugam123 commented 2 years ago

We tried couple of experiments:

eocanha commented 2 years ago

The http-headers event is needed when playing adaptive streaming (SmoothStreaming, HLS, DASH) regular non-MSE video.

It's used in this custom patch to configure the internal GstSoupHttpSrc elements with cookies: https://github.com/WebPlatformForEmbedded/buildroot/blob/main/package/gstreamer1/gst1-plugins-bad/1.16.2/0001-adaptivedemux-minimal-HTTP-context-support.patch#L40   <-- This way of dealing with headers is based on context queries, which is a mechanism different to events.

It's used by uridownloader here: https://github.com/GStreamer/gst-plugins-bad/blob/master/gst-libs/gst/uridownloader/gsturidownloader.c#L203

eocanha commented 2 years ago

In the end, it looks like the http-headers custom downstream sticky event may also be important to supply the Content-Length header internally and help the GstDownloadBuffer to do its job internally (when used, and mainly on non-adaptive streaming content), so the sending of this event can't be avoided.

I couldn't reproduce the problem locally myself, but having a look at the code that sends the event, it looks like the root issue of this deadlock is that we're using the main thread to perform an action (sending the event) that is meant to actually happen from the appsrc's streaming thread. The thread is blocked until the downstream elements finish processing the remaining buffers (what is stored in the big queue you mentioned).

The right solution here would be to backport the patch that implements the feature of offline event sending in appsrc, which is already present upstream. With this behaviour, the event is queued in the appsrc, in series with any GstBuffer that may also be pending to be pushed downstream, and when its turn comes, appsrc will send the event.

I'll work in the coming days to backport the code.

eocanha commented 2 years ago

It turns out that the support for proper event sending in appsrc is tangled with other features, including segment event management, and isn't as easily backportable as I initially thought.

In the end, I implemented a solution in the https://github.com/WebPlatformForEmbedded/WPEWebKit/commits/eocanha/eocanha-113 branch using probes to emit the event in the streaming thread. It ensures that the custom event is emitted only after the segment event.

Test that it fixes the deadlock for you and, if it does, I can merge it in the wpe-2.22 branch.

eocanha commented 2 years ago

Pushed on wpe-2.22 as commit https://github.com/WebPlatformForEmbedded/WPEWebKit/commit/38adbc589864851346aa3e4156723b5d94cc254d after Vivek validated that the original issue is no longer present.

varumugam123 commented 2 years ago

Thank you @eocanha