559217 - Browser freezes due a lock contention across Places statements

Status: RESOLVED FIXED

(Toolkit :: Places, defect)

Description

[Justin Dolske [:Dolske]]

Attachment: process sample output

I seem to be getting these freezes once or twice a day. Seems to happen most commonly shortly after starting the browser (or opening a bunch of tabs?), and maybe on shutdown. Samples have a similar pattern, usually docshell/history stuff is resetting a statement.

These freezes usually last 15-30 seconds, which is really bad. :(

Comment 1

[Justin Dolske [:Dolske]]

Attachment: Another hang

This was maybe 30 seconds after restarting the browser, just 2 tabs, both had restored. I was clicking the amusing "did you mean recursion" link when you Google recursion, and after 2 or 3 clicks it froze again. My places.sqlite file is 65006592 bytes in size.

Comment 2

[Shawn Wilsher :sdwilsh]

(In reply to comment #1)
> This was maybe 30 seconds after restarting the browser, just 2 tabs, both had
> restored. I was clicking the amusing "did you mean recursion" link when you
> Google recursion, and after 2 or 3 clicks it froze again. My places.sqlite file
> is 65006592 bytes in size.
We should probably make this a pure async statement.  We have a lock contention issue there.

Comment 3

[Justin Dolske [:Dolske]]

Argh, these hangs are driving me crazy. :( sdwilsh, you said this was a simple fix, could you roll up a patch or reassign?

Comment 4

[Marco Bonardo [:mak]]

i'm unsure is Shawn was pointing to registerOpenPage, but even if it's sync that is writing to a temp table, it should not cause any hang. Having a list of the statements that are running in that moment would be great.

i'd like to know what is this mozilla::storage::AsyncExecuteStatements::Run(). 

Make a backup of your places.sqlite so we don't lose the opportunity to reproduce this.

Let's start chcking expiration:
could you tell me the value of places.history.expiration.transient_current_max_pages

Then run in the error console
Components.utils.import("resource://gre/modules/PlacesDBUtils.jsm");PlacesDBUtils.checkAndFixDatabase();

and post the log that will come out after some seconds

Comment 5

[Shawn Wilsher :sdwilsh]

The problem is the call to Reset on the main thread, which is called by ExecuteAsync (due to a scoper: http://mxr.mozilla.org/mozilla-central/source/toolkit/components/places/src/History.cpp#80).  We really need to abandon the scoper if we've succeeded everything, so this turns into a two liner.

Comment 6

[Marco Bonardo [:mak]]

yeah that scoper should not be there, good catch

Comment 7

[Marco Bonardo [:mak]]

Stealing bug, Shawn will just review.
I'm actually touching the same code to remove Storage deprecate APIs usage, thus since that bug is in review i have to enqueue things to aviod bitrotting.

Comment 8

[Marco Bonardo [:mak]]

Attachment: patch v1.0

Comment 9

[Marco Bonardo [:mak]]

http://hg.mozilla.org/mozilla-central/rev/5798a44e9184