Opened 7 years ago

Closed 7 years ago

#5710 closed defect (fixed)

Fix/Report SessionStore leakage

Reported by: mikeperry Owned by: mikeperry
Priority: Very High Milestone:
Component: Firefox Patch Issues Version:
Severity: Keywords: MikePerry201205
Cc: Shondoit, g.koppen@… Actual Points: 2
Parent ID: Points: 2
Reviewer: Sponsor:

Description

In #4430, Some Guy is quite insistent that the session store still leaks in Windows, and has a repro case.

S/he claims that all you have to do is close Tor Browser with a tab open, and it will write the title of that tab to sessionstore.js

I think this is a Firefox bug. I wonder if it happens in Private Browsing Mode. I guess I need to unmothball my Windows VM and see if I can repro, too.

Child Tickets

Attachments (2)

torbutton-1.4.6pre1.xpi (756.0 KB) - added by mikeperry 7 years ago.
Should prevent sessionstore.js from being created in torbrowser
torbutton-1.4.6pre2.xpi (756.0 KB) - added by mikeperry 7 years ago.
Fix for annoying resize issue, too

Download all attachments as: .zip

Change History (14)

comment:1 Changed 7 years ago by cypherpunks

If the problem is somewhere in JS and you know what code paths get executed when the browser closes, I could install Venkman and try to set breakpoints here and there.

comment:2 Changed 7 years ago by Shondoit

Cc: Shondoit added

comment:3 Changed 7 years ago by gk

Cc: g.koppen@… added

comment:4 Changed 7 years ago by Shondoit

I can confirm this behaviour.
It consistenly shows the title of the tab active during closing in sessionstore.js, even if that tab is on https. No other titles are stored and no urls at all are stored.

Private browsing does not store the title when closed normally.

I tried playing with the browser.sessionstore.* settings, but this did not seem to effect anything.
Torbutton did not seem to overwrite the following settings since above described behaviour was also observed using stock Mozilla Firefox.

I used the following settings:
browser.sessionstore.max_resumed_crashes = 0
browser.sessionstore.max_tabs_undo = 10
browser.sessionstore.max_windows_undo = 3
browser.sessionstore.privacy_level = 2
browser.sessionstore.privacy_level_deferred = 2

More importantly though: I can reproduce this exact behaviour in Ubuntu 12.04 as well.
It means you can start testing in your favourite OS and revert to Windows when needed.

comment:5 Changed 7 years ago by mikeperry

cypherpunks: Pretty sure the nsSessionStore.js file has all the observers that cause it to write stuff at various browser events (such as final-ui-shutdown, etc). Though, what might be happening is that something else is *also* listening on those observers and calling in to the nsSesionStore.js? But either way, breakpoints will help get to the bottom of that.

In the source tree, the path to it is ./browser/components/sessionstore/src/nsSessionStore.js.

I will be working on #5715 right now, so anything you discover will be helpful.

Shondoit: Hrmm.. I still can't reliably reproduce it. Note TBB also sets browser.sessionstore.resume_on_crash to false. Though the private browsing mode info is a useful datapoint to check the source for. Perhaps the source file does something special if PMB is enabled that it doesn't do for resume_on_crash=false.

comment:6 Changed 7 years ago by mikeperry

Summary: Fix/Report SessionStore leakage on WindowsFix/Report SessionStore leakage

M-Fer. My SessionStore just shat itself on Linux TBB.. Not sure what triggered it, the browser hasn't even been closed, but everything I've done is in sessionstore.js.. Even full urls are present. It's as if both the torbutton filters and the pref both completely failed for some reason.

This is a nightmare of unreproducible non-determinism.

comment:7 in reply to:  5 Changed 7 years ago by cypherpunks

Replying to mikeperry:

cypherpunks: Pretty sure the nsSessionStore.js file has all the observers that cause it to write stuff at various browser events (such as final-ui-shutdown, etc).

This isn't very helpful, nsSessionStore.js is >4000 lines long and it doesn't contain the string "final-ui-shutdown". I was able to find the function "sss_saveStateObject" in /FirefoxPortable/App/Firefox/components/nsSessionStore.js and to see if I can get things going I put some code inside the function body to spawn a random exe. All worked as expected and I saw the exe launched during shutdown, but to my total amazement I can't get rid of this hack now. I have restored the original nsSessionStore.js without my app-launching code and it still launches the exe on shutdown. WHAT THE FUCK? Some caching mechanism that I'm unaware of?

Anyway, if you could provide some code to dump the stack and other information necessary to diagnose what and why calls this function, that would be helpful. Perhaps you could just chuck out sss_saveStateObject and be done with this bug, removing scary stuff is always safer than playing whack-a-mole with preferences and fancy filters.

comment:8 Changed 7 years ago by mikeperry

Wow, that XPCOM component caching would be news to me. I've always been able to edit the things in place and have it work out, at least for extension components..

As for your brutal hack approach, I am thinking you're right.. However, I also want to remove the session store observer in torbutton.. I could possibly just hack that to filter everything out in the case of Tor Browser, instead. I'm going to try that right now first. If that observer is also totally broken, telling Mozilla about it might be helpful, I guess...

comment:9 Changed 7 years ago by mikeperry

Ok, I think this is fixed. I altered the filter to completely remove all session store data so that sessionstore.js is no longer written to disk at all. With how much of a pain this bug has been, I'd really appreciate if someone could test this on Windows for a while.

You can build the xpi from a git checkout of torbutton with:

git clone git://git.torproject.org/git/torbutton.git
cd torbutton
./makexpi.sh

It should show up in pkg.

I'll also attach an XPI to this bug.

Changed 7 years ago by mikeperry

Attachment: torbutton-1.4.6pre1.xpi added

Should prevent sessionstore.js from being created in torbrowser

comment:10 Changed 7 years ago by mikeperry

Two caveats: 1. You of course need to delete your existing sessionstore.js, it won't delete it for you. 2. I think my fix for #5729 is causing that XPI to mess with my window sizes when I open new tabs...

Changed 7 years ago by mikeperry

Attachment: torbutton-1.4.6pre2.xpi added

Fix for annoying resize issue, too

comment:11 Changed 7 years ago by mikeperry

Ok, I attached an xpi with a fix for the resizing problem introduced by #5729's fix. Hopefully this will make it less annoying to test for long periods of time (to ensure the sessionstore.js doesn't show up through some other unknown black magic).

comment:12 Changed 7 years ago by mikeperry

Actual Points: 2
Points: 2
Resolution: fixed
Status: newclosed

Calling this fixed. Please reopen if you notice otherwise w/ the above test xpi.

Note: See TracTickets for help on using tickets.