nsConsoleService posts LogMessageRunnables to the event queue uselessly even when there are no observers

RESOLVED FIXED in mozilla18

Status

()

Core
XPCOM
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: seth, Assigned: seth)

Tracking

Trunk
mozilla18
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [MemShrink:P3])

Attachments

(2 attachments)

(Assignee)

Description

5 years ago
Created attachment 665615 [details]
Sample web page that triggers the bug by generating huge numbers of CSS errors during the parsing of an SVG file built from a long data URI.

nsConsoleService posts LogMessageRunnables to the event queue uselessly even when there are no observers. This results in huge amounts of memory being retained uselessly until the LogMessageRunnables execute. If there are very many or very large messages (or both, in the worst case) this can easily push the system into swap.
(Assignee)

Updated

5 years ago
Assignee: nobody → seth
(Assignee)

Updated

5 years ago
See Also: → bug 786108
Whiteboard: [MemShrink:P3]
(Assignee)

Comment 1

5 years ago
I've got a patch already which I'm preparing to upload now.
Status: NEW → ASSIGNED
To elaborate slightly: Quoting Neil from bug 786108 comment #8...
> > There are 892 instances of the string "writing-mode" in the encoded SVG
> > document, each of which triggers a warning-message with a ~1 MB URL embedded
> > in it.  So that's 1 GB of memory right there.
> 
> Fortunately we only keep the last 250 console messages.

So, as Seth said in comment 0 here, we create a runnable for every errors to be posted to the error console, and the runnable keeps its URI-string alive until it's executed.  These _runnables_ aren't capped to 250 (though the actual list of error-console-messages is).

So, while we parse a stylesheet, we can generate an unbounded number of these runnables in the event queue and hence generate an unbounded number of copies of the URI-string.  (After bug 786108, they should all share the same URI-string, but it's still worth avoiding them in the first place if we can.)
OS: Mac OS X → All
Hardware: x86 → All
(Assignee)

Comment 3

5 years ago
Created attachment 665624 [details] [diff] [review]
Bug 795086 - Make nsConsoleService only post a LogMessageRunnable to the event queue if there are any observers for it.

Lazily create and launch the LogMessageRunnable only if we have observers. Shows massive reduction in memory usage on the test case; before it was entering swap on a machine with 16 GB of RAM, while with this patch it tops out at ~1 GB.
Attachment #665624 - Flags: feedback?(bzbarsky)
(Assignee)

Comment 4

5 years ago
Indeed; bug 786108 greatly reduces the impact of this issue for this specific test case because of sharing between the URI strings, but even with that fix in place, a huge number of unnecessary LogMessageRunnables are still created. And, of course, this may happen in other circumstances that bug 786108 doesn't address, so it's worth fixing this issue directly.
(--> Clearing "See Also" field -- that's reserved for "duplicates of this bug in external bug trackers" (e.g. in the Ubuntu bug-tracker or in the bug-tracker of a tool that we're interacting with).  See the hover-text for that field for more info.)
See Also: bug 786108
(Assignee)

Comment 6

5 years ago
Ah, whoops, thanks.
Comment on attachment 665624 [details] [diff] [review]
Bug 795086 - Make nsConsoleService only post a LogMessageRunnable to the event queue if there are any observers for it.

r=me.
Attachment #665624 - Flags: review+
Attachment #665624 - Flags: feedback?(bzbarsky)
Attachment #665624 - Flags: feedback+
Try run: https://tbpl.mozilla.org/?tree=Try&rev=b5fec0d9d9e9
There was some infra-related purple/red in that try run, but there was enough green (and the change is minimally-invasive-enough) that I don't think we need to worry about missing those jobs.

Pushed to inbound:
  https://hg.mozilla.org/integration/mozilla-inbound/rev/8e0593f974bb
https://hg.mozilla.org/mozilla-central/rev/8e0593f974bb
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla18
You need to log in before you can comment on or make changes to this bug.