1374013 - Hang and memory consumption, for testcase that updates style element inside of onload handler for that same element

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[Tyson Smith [:tsmith]]

Attachment: test_case.html

The attached test case seems to hang the browser. I let it run for 10 minutes on an opt build but it did not exit. It seems to slowly consume memory. I have seen memory usage as high as 5GB.

Comment 1

[Timothy Nikkel (:tnikkel)]

Every time we set the textContent of the script element we call onload on the script element so we keep calling it's onload callback. Chrome doesn't seem to do this.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

In the process of migrating remaining bugs to the new severity system, the severity for this bug cannot be automatically determined. Please retriage this bug using the new severity system.

Comment 3

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)]

According to the spec, when <style> element is updated, load event should be fired synchronously.

1. Let element be the style element associated with the style sheet in question.
2. ...
3. ...
4. Queue an element task on the networking task source given element and the following steps:
   1. If success is true, fire an event named load at element.
      ...

The element must delay the load event of the element's node document until all the attempts to obtain the style sheet's critical subresources, if any, are complete.

Therefore, I think that Gecko's behavior conforms to the spec, however, from the security point of view, the load event should be dispatched asynchronously in this situation or just stop queuing new load event in this case.

smaug: Could you set the severity at least?

Comment 4

[Olli Pettay [:smaug][[e-mail]]]

That load event firing is from css side.

Comment 5

[Daniel Holbert [:dholbert]]

Here's a profile that I captured of the first 15 seconds of me viewing the testcase (as it loads/hangs forever): https://share.firefox.dev/3o5QX5V

Triaging as S3, on the assumption that this is a non-exploitable DoS type issue.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to Olli Pettay [:smaug][[e-mail]] from comment #4)

That load event firing is from css side.

Specifically, the load event is dispatched here in mozilla::css::SheetLoadData::FireLoadEvent, as shown in the performance profile:

https://searchfox.org/mozilla-central/rev/45b20d00311bc2170dfbdae8a91bc4dca29717d8/layout/style/Loader.cpp#447-463

void SheetLoadData::FireLoadEvent(nsIThreadInternal* aThread) {
  // First remove ourselves as a thread observer.  But we need to keep
  // ourselves alive while doing that!
  RefPtr<SheetLoadData> kungFuDeathGrip(this);
  aThread->RemoveObserver(this);

  // Now fire the event.
  //
  // NOTE(emilio): A bit weird that we fire the event even if the node is no
  // longer in the tree, or the sheet that just loaded / errored is not the
  // current node.sheet, but...
  nsCOMPtr<nsINode> node = std::move(mOwningNodeBeforeLoadEvent);
  MOZ_ASSERT(node, "How did that happen???");

  nsContentUtils::DispatchTrustedEvent(node->OwnerDoc(), node,
                                       mLoadFailed ? u"error"_ns : u"load"_ns,
                                       CanBubble::eNo, Cancelable::eNo);

That event gets synchronously handled, and updates the stylesheet (mozilla::dom::HTMLStyleElement::SetTextContentInternal). Several stack levels inside of that, we trigger mozilla::css::SheetLoadData::ScheduleLoadEventIfNeeded which calls internalThread->AddObserver(this)) (which I think is scheduling the load event to be dispatched) and mLoader->BlockOnload() (which, combined with the ping-pong effect, is why we never end up unblocking onload).

Comment 7

[Daniel Holbert [:dholbert]]

Attachment: testcase 2: simplified slightly, and then added logging and a limit

For convenience, here's a testcase where I've added some console.log logging to track the iteration count, and I've added a hardcoded 5-second limit to the hang (removing the onload handler when that much time has passed).

Comment 8

[Daniel Holbert [:dholbert]]

I'm not sure if it's correct or not, but in Chrome the attached testcase 2 only hits the load handler twice. i.e. console.log shows:

Iteration number 0
Iteration number 1

...and that's it.

In Firefox, the logging continues indefinitely until we reach the 5 second safety limit that I've baked into that testcase.

Comment 9

[Daniel Holbert [:dholbert]]

(WebKit essentially matches Firefox here, for what it's worth. Epiphany (WebKit-based browser on Linux) does stop playing the page-loading animation, but they peg a CPU core indefinitely, for the original testcase, and they hang and continue console.log()'ing iterations until 5 seconds elapses in my testcase 2.)