Closed Bug 934935 Opened 11 years ago Closed 11 years ago

Facebook.com spams sessionstore.js

Categories

(Tech Evangelism Graveyard :: Other, defect)

Product:
Tech Evangelism Graveyard
Component:
Other
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

(Not tracked)

Status:
RESOLVED FIXED

People

(Reporter: Yoric, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: perf, Whiteboard: [MemShrink:P1])

Attachments

(3 files, 3 obsolete files)

2013-11-24 16_24_19-about_sessionstore.png
2.78 KB, image/png
Details
'switching advertisements system'
39.94 KB, image/png
Details
2013-11-28 17_07_56-about_sessionstore after fix
4.59 KB, image/png
Details 
In bug 669034, C wrote:
« So, Firefox 24.0, I only have a reasonable number of tabs, and used to see stalls of 3000 ms (yes, 3 seconds) in the parent of ssi_serializeHistoryEntry(), as well as 800 ms to write the sessionstore.js file to disk. I have a quad-core Haswell Xeon and a fast Intel SSD. The sessionstore.js file was 60MB.

I tracked it down to some ajax-y query mechanism Facebook chat uses that leaks long (>1kB) urls which all get persisted.

From a little python script I wrote to sort strings in the JSON by length, outputting keys:
1212 /windows[0]/tabs[5]/entries[5]/children[14]/url
1212 /windows[0]/tabs[5]/entries[5]/children[4]/url
1212 /windows[0]/tabs[5]/entries[5]/children[8]/url

These URLs are of the form "https://www.facebook.com/ai.php?something=1kB_of_gobblydegook". The result of GETing this 1kB URL is a 0.10kB response:

<span id="fbEmuTrackingSucc&#101;ss">Succ&#101;ss</span>

Those leaking URLs accumulated 10-50kB per minute towards my sessionstore.js, eventually leading to 3000 ms stalls every 10 seconds, which is about as fun as it sounds. I used adblock to block the URLs, and now my sessionstore.js is fairly stable at around 100-120 kB.

I'm not sure what can be done from the Firefox side to garbage collect this sort of behavior (it's hard to destroy manually without losing your session because of tab-undo); it could certainly be fixed on Facebook's end, but their public bug-filing mechanism is pretty bad.
»
I was able to reproduce this problem myself. Here's a rough STR:

1. Log into Facebook.
2. Turn off adblock for facebook.com. Adblock eliminates the problem for me.
3. Click on the messages button in the top blue bar. The button looks like two cartoon speech bubbles.
4. Click on "See all".
5. Click on "+ New Message".
6. For the "To:" field, enter yourself (or someone you want to annoy).
7. Enter a bunch of messages. Wait for ads to display on the right hand side.

Fairly quickly, you'll accumulate a bunch of history entries. As far as I can tell, sending a message (i.e., hitting the blue reply button) adds a new entry to the history, probably via history.pushState. However, the number of entries is capped at 50. (I'm not sure if the cap is in Gecko or session restore.)

As you're sending messages, new ads get displayed. Each ad adds a new iframe, and I don't think they ever go away. After spending about 15 minutes on the page, if I open the Web Console and print window.frames.length, I get 55. This number just keeps increasing over time.

The really awful part is that our saved session store data ends up looking like this:

windows[0].tabs[0].entries[0].children = [A, B, C]
windows[0].tabs[0].entries[1].children = [A, B, C, D, E, F]
windows[0].tabs[0].entries[2].children = [A, B, C, D, E, F, G, H, I]
...

Notice that the ad frames like A that are added early-on get duplicated in each history entry--up to 50 times. The data for each frame includes a very long URL, so this is why the sessionstore.js file gets so huge.

Firefox could sort of work around this problem by trying to share the data for frames that are the same across history entries. In the example above, we should be writing out only 1 copy of A instead of 50 copies. That would be a major change to the session restore code, though, and it would have to spend time on Aurora and beta. It also wouldn't completely solve the problem--we'd still be writing out data for tons of ads that haven't been visible for a very long time.

It would be better if Facebook fixed the problem by eliminating the iframes for ads that are no longer displayed. It looks like at most four ads are shown at one time. Even multiplied by 50 history entries, that's only 200 ad URLs to save.
(In reply to Bill McCloskey (:billm) from comment #1)
> It would be better if Facebook fixed the problem by eliminating the iframes
> for ads that are no longer displayed. 

Yes, but Firefox should detect this somehow and prevent the sessionstore from being spammed – after all, other sites could do the same. 

So IIRC, Firefox is removing the iFrame from display but keeps a reference to it in JS. And sessionstore cannot detect that this iFrame is no longer displayed and just drop it?
I was talking to Olli on IRC today and it sounds like it might be okay to avoid storing any data for dynamically-generated frames. The history code makes it pretty easy to check for this:

http://mxr.mozilla.org/mozilla-central/source/docshell/shistory/public/nsISHEntry.idl?mark=220-230#200

There are two approaches we could try:
1. Don't store any information for child frames if there are any dynamically generated frames on the page.
2. Skip dynamically generated frames when serializing entries.

Here's a page we can test with: http://mozilla.pettay.fi/moztests/history6/

This would fix the session store problems with Facebook. It won't fix the memory leak aspect.
I've noticed Facebook is making nightly unresponsive is this related to this bug?
(In reply to Bill McCloskey (:billm) from comment #3)
> I was talking to Olli on IRC today and it sounds like it might be okay to
> avoid storing any data for dynamically-generated frames.

+1

Thanks for reporting the memory leak, I will fix this on our side.
(In reply to Benjamin Kerensa from comment #5)
> I've noticed Facebook is making nightly unresponsive is this related to this
> bug?

If your sessionrestore file is huge and filled with Facebook URLs, then it's likely related. If not, please file a new bug.
> I'm not sure what can be done from the Firefox side to garbage collect this
> sort of behavior (it's hard to destroy manually without losing your session
> because of tab-undo); it could certainly be fixed on Facebook's end, but
> their public bug-filing mechanism is pretty bad.

Yes, they have no Bugzilla. :D

But you can report bugs here:
https://www.facebook.com/help/326603310765065/

And give an feedback here:
https://www.facebook.com/help/127103474099499/

I did this for this bug.
(In reply to Florian Bender from comment #2)
> Yes, but Firefox should detect this somehow and prevent the sessionstore
> from being spammed – after all, other sites could do the same. 

Move to:
Product: 	Firefox
Component: 	Session Restore
???
I have reached out to contacts at Facebook, we'll see if it works.
Tobias: the apparent bug is at Facebook.com, so we'll leave it in Tech Evangelism. Counter-measures, of course, should go in Session Restore.
Facebook here.  I've opened a task and will see if I can get to the bottom of this on our end.
(In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment #0)
> it could certainly be fixed on Facebook's end, but
> their public bug-filing mechanism is pretty bad.

David, please feel free to reach out to me personally in the future (philikon@fb.com). In this case, Johnny pinged me via email which got us to look at the issue very quickly.
MemShrink:P1 because (a) this can cause high memory consumption (as seen in bug 934931), and (b) I hear Facebook is quite a popular site.
Whiteboard: [MemShrink:P1]
Thanks Robert, Philip. If there is any way we can help, don't hesitate to ping us.
Here is a screenshot of my sessionstore-infos that I have about a fb-message-page.
Taken from this extension:
https://addons.mozilla.org/en-US/firefox/addon/about-sessionstore/
Just another screenshot of the sessionstore.js data with an open messenger page.
There is a rumor that Facebook has fixed this issue at some point during the week.
billm (or someone else), could you check?
Flags: needinfo?(wmccloskey)
(In reply to David Rajchenbach Teller [:Yoric] <needinfo? me> from comment #20)
> There is a rumor that Facebook has fixed this issue at some point during the
> week.
> billm (or someone else), could you check?

Looks good for me.

Philipp Weitershausen, should we close this bug?
Flags: needinfo?(wmccloskey) → needinfo?(philipp)
OK, still not fixed!
And I think it comes from the 'switching advertisements system' and it's not only related to the message page.
Attachment #831778 - Attachment is obsolete: true
Attachment #832037 - Attachment is obsolete: true
Attachment #8336703 - Attachment is obsolete: true
Flags: needinfo?(philipp)
There's a diff going out this week (tomorrow afternoon) that should fix this on our end.
Thanks, Robert.
Does this fix address the garbage-collection of iframes or the 10kb long URIs?
Flags: needinfo?(broofa)
Tobias, could you confirm that the fix works on our side?
Flags: needinfo?(Tobias.Besemer)
Ok, that looks better. I guess we can close this bug.
Thanks everyone.
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Looks good for me! (See: Comment 27)

But I will have an look at it over the next days ...
Flags: needinfo?(Tobias.Besemer)
David, the fix takes care of leaking iframes.  'Doesn't do anything for URI length.
Flags: needinfo?(broofa)
QA Contact: Tobias.Besemer
Product: Tech Evangelism → Tech Evangelism Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: