Marco Bonardo [:mak] Reporter
	Description • 15 years ago

This happens in Places but i'm filing under storage since it's an sqlite assertion and is probably caused by concurrent sync and async queries

Replacing UNION ALL with UNION in mDBGetChildren query, that is executed by nsNavBookmarks::QueryFolderChildren solves the assertion, so i think there is a transaction that exists before the query runs, but is removed between the first and the second part of the UNION ALL. Not shure how that's possible though.

Here is the stack when i hit the assertion, i can usually reproduce running tests in toolkit/components/places/tests/bookmarks/ but there is also a Gentoo bug filed for a similar issue: http://bugs.gentoo.org/281695
In the Gentoo case they were/are probably using system SQLite, but i'm on Windows and i reproduces with a completely clobber build (VS2008 + Windows7 sdk).

 msvcr90d.dll!1023c355()     
     [Frames below may be incorrect and/or missing, no symbols loaded for
msvcr90d.dll]    
     msvcr90d.dll!102de9ad()     
     msvcr90d.dll!102cf377()     
>	xpc3250.dll!DEBUG_CheckWrapperThreadSafety(const XPCWrappedNative * wrapper=0x00d20000)  Line 3677 + 0x29 bytes	C++
     msvcr90d.dll!102c103e()     
     msvcr90d.dll!1023d496()     
     msvcr90d.dll!102cff17()     
     msvcr90d.dll!102cff0e()     
     msvcr90d.dll!102c2ee8()     
     msvcr90d.dll!102c2edd()     
     msvcr90d.dll!102cff17()     
     msvcr90d.dll!102cff0e()     
     msvcr90d.dll!102cfb2f()     
     msvcr90d.dll!102cfadc()     
     msvcr90d.dll!102db25b()     
     sqlite3.dll!sqlite3MemMalloc(int nByte=384)  Line 12540 + 0xd bytes    C
     sqlite3.dll!sqlite3Malloc(int n=0)  Line 15873 + 0xa bytes    C
     sqlite3.dll!sqlite3DbMallocRaw(sqlite3 * db=0x01df5dfc, int n=31416024) 
Line 16178 + 0x9 bytes    C
     sqlite3.dll!btreeCursor(Btree * p=0x016d4ea0, int iTable=2, int wrFlag=0,
KeyInfo * pKeyInfo=0x00000000, BtCursor * pCur=0x0204dd28)  Line 40843 + 0x20
bytes    C
     sqlite3.dll!sqlite3BtreeCursor(Btree * p=0x016d4ea0, int iTable=2, int
wrFlag=0, KeyInfo * pKeyInfo=0x00000000, BtCursor * pCur=0x0204dd28)  Line
40880 + 0x19 bytes    C
     sqlite3.dll!sqlite3VdbeExec(Vdbe * p=0x016b2830)  Line 54777 + 0x2a bytes 
  C
     sqlite3.dll!sqlite3Step(Vdbe * p=0x016b2830)  Line 50423 + 0x9 bytes    C
     sqlite3.dll!sqlite3_step(sqlite3_stmt * pStmt=0x016b2830)  Line 50484 +
0x9 bytes    C
     storagecomps.dll!mozilla::storage::Statement::ExecuteStep(int *
_moreResults=0x0012ef04)  Line 713 + 0xc bytes    C++
     places.dll!nsNavBookmarks::QueryFolderChildren(__int64 aFolderId=3,
nsNavHistoryQueryOptions * aOptions=0x02046cf8,
nsCOMArray<nsNavHistoryResultNode> * aChildren=0x0204b470)  Line 2417 + 0x26
bytes    C++
     places.dll!nsNavHistoryFolderResultNode::FillChildren()  Line 3373 + 0x2c
bytes    C++
     places.dll!nsNavHistoryFolderResultNode::OpenContainer()  Line 3193 + 0x8
bytes    C++
     places.dll!nsNavHistoryContainerResultNode::SetContainerOpen(int
aContainerOpen=1)  Line 550    C++
     places.dll!nsNavHistoryFolderResultNode::SetContainerOpen(int
aContainerOpen=1)  Line 763 + 0x10 bytes    C++
     xpcom_core.dll!NS_InvokeByIndex_P(nsISupports * that=0x00000012, unsigned
int methodIndex=1, unsigned int paramCount=1241672, nsXPTCVariant *
params=0x0204ccd0)  Line 102    C++
     xpc3250.dll!XPCWrappedNative::CallMethod(XPCCallContext & ccx={...},
XPCWrappedNative::CallMode mode=CALL_SETTER)  Line 2710 + 0x21 bytes    C++
     xpc3250.dll!XPCWrappedNative::SetAttribute(XPCCallContext & ccx={...}) 
Line 2527 + 0xe bytes    C++
     xpc3250.dll!XPC_WN_GetterSetter(JSContext * cx=0x0164dcd8, JSObject *
obj=0x02011440, unsigned int argc=1, int * argv=0x0164f5f8, int *
vp=0x0012f51c)  Line 1776 + 0xc bytes    C++
     js3250.dll!js_Invoke(JSContext * cx=0x0164dcd8, unsigned int argc=1, int *
vp=0x0164f5f0, unsigned int flags=2)  Line 1363 + 0x1a bytes    C++
     js3250.dll!js_InternalInvoke(JSContext * cx=0x0164dcd8, JSObject *
obj=0x02011440, int fval=33625184, unsigned int flags=0, unsigned int argc=1,
int * argv=0x0012fcb4, int * rval=0x0012fcb4)  Line 1426 + 0x15 bytes    C++
     js3250.dll!js_InternalGetOrSet(JSContext * cx=0x0164dcd8, JSObject *
obj=0x02011440, int id=22162316, int fval=33625184, JSAccessMode
mode=JSACC_WRITE, unsigned int argc=1, int * argv=0x0012fcb4, int *
rval=0x0012fcb4)  Line 1462 + 0x1f bytes    C++
     js3250.dll!JSScopeProperty::set(JSContext * cx=0x0164dcd8, JSObject *
obj=0x02011440, int * vp=0x0012fcb4)  Line 810 + 0x23 bytes    C++
     js3250.dll!js_NativeSet(JSContext * cx=0x0164dcd8, JSObject *
obj=0x02011440, JSScopeProperty * sprop=0x0174b678, bool added=false, int *
vp=0x0012fcb4)  Line 4349 + 0x14 bytes    C++
     js3250.dll!js_SetPropertyHelper(JSContext * cx=0x0164dcd8, JSObject *
obj=0x02011440, int id=22162316, unsigned int defineHow=1, int * vp=0x0012fcb4)
 Line 4746 + 0x1a bytes    C++
     js3250.dll!js_Interpret(JSContext * cx=0x0164dcd8)  Line 1884 + 0x1f bytes
   C++
     js3250.dll!js_Execute(JSContext * cx=0x0164dcd8, JSObject *
chain=0x01cc2b60, JSScript * script=0x016650b8, JSStackFrame * down=0x00000000,
unsigned int flags=0, int * result=0x0012fe08)  Line 1598 + 0x9 bytes    C++
     js3250.dll!JS_EvaluateUCScriptForPrincipals(JSContext * cx=0x0164dcd8,
JSObject * obj=0x01cc2b60, JSPrincipals * principals=0x015e9e9c, const unsigned
short * chars=0x016681c8, unsigned int length=16, const char *
filename=0x0041864c, unsigned int lineno=1, int * rval=0x0012fe08)  Line 5056 +
0x19 bytes    C++
     js3250.dll!JS_EvaluateScriptForPrincipals(JSContext * cx=0x0164dcd8,
JSObject * obj=0x01cc2b60, JSPrincipals * principals=0x015e9e9c, const char *
bytes=0x00d2f01d, unsigned int nbytes=16, const char * filename=0x0041864c,
unsigned int lineno=1, int * rval=0x0012fe08)  Line 5020 + 0x25 bytes    C++
     xpcshell.exe!ProcessArgs(JSContext * cx=0x0164dcd8, JSObject *
obj=0x01cc2b60, char * * argv=0x00d2ed2c, int argc=14)  Line 1193 + 0x3d bytes 
  C++
     xpcshell.exe!main(int argc=14, char * * argv=0x00d2ed2c, char * *
envp=0x00d24038)  Line 1879 + 0x15 bytes    C++
     xpcshell.exe!__tmainCRTStartup()  Line 586 + 0x19 bytes    C
     xpcshell.exe!mainCRTStartup()  Line 403    C
     kernel32.dll!7c817077()     
     js3250.dll!NativeToValueBase<FailDoubleOOMHandler>(JSContext *
cx=0xcccccccc, int & v=, JSTraceType_ type='Ì', double * slot=0xcccccccc)  Line
2690 + 0x7 bytes    C++

	Daniel Holbert [:dholbert]
	Comment 1 • 15 years ago

Currently, I hit this assertion (and subsequently abort) at startup-time, whenever I run an up-to-date mozilla-central debug build on my personal profile.  This happens before any Firefox windows appear.

Attaching backtrace.

	Marco Bonardo [:mak] Reporter
	Comment 2 • 15 years ago

i just tried to apply sqlite 3.6.19 to my tree, and it did not solve this.

	Marco Bonardo [:mak] Reporter
	Comment 3 • 15 years ago

a places test that can reproduce this is test_429505_remove_shortcuts.js

	Marco Bonardo [:mak] Reporter
	Comment 4 • 15 years ago

this solves the assertion.

what looks like is happening is that an async stmt creates an immediate transaction, first part of mDBGetChildren runs, then the async stmt ends and clear the transaction, and second part of mDBGetChildren runs, but while it started in a transaction it ends out of it.

I don't know if this hacky workaround can also solve the crash in bug 520541, could be interesting to know though.

	D. Richard Hipp
	Comment 5 • 15 years ago

Thanks for the clues.  We now have a reproducible test case in SQLite.
See http://www.sqlite.org/src/info/f777251dc7 for details.  We will get
this patched for you as soon as we can.

	Marco Bonardo [:mak] Reporter
	Comment 6 • 15 years ago

thank you, i really appreciate that :)

	(not currently active) Ted Mielczarek
	Comment 7 • 15 years ago

test_bookmarks.js is failing consistently with this error on Linux debug unittests.

	Marco Bonardo [:mak] Reporter
	Comment 8 • 15 years ago

if we don't have a clear timeline for sqlite 3.6.20 we can eventually land my workaround in the meantime, and back it out as soon as we update sqlite.

	Marco Bonardo [:mak] Reporter
	Comment 9 • 15 years ago

or backout sqlite 3.6.18 and go back to 3.6.16, i think shawn was thinking to that.

	D. Richard Hipp
	Comment 10 • 15 years ago

SQLite version 3.6.20 will be early-to-mid November.  But we are happy to push out a patch release (3.6.19.1) containing only the bug fixes that impact FF.  We can do that in about a day.  All you have to do is ask and it will be done.

	(not currently active) Ted Mielczarek
	Comment 11 • 15 years ago

This is causing test_bookmarks.js to fail on debug unittests.

	Marco Bonardo [:mak] Reporter
	Comment 12 • 15 years ago

If Shawn is ok with that, i think there's no need to rush an sqlite version from sqlite team, we should instead just backout 3.6.18 on central and wait for .20, we don't rely on any new feature that was not present in 3.6.16.

	D. Richard Hipp
	Comment 13 • 15 years ago

Another option is to simply use intermediate SQLite check-ins.

Historically, FF has only used official SQLite releases.  This was a good policy because it made it very clear exactly which SQLite version was being used within FF.  But SQLite has now shifted configuration management from CVS to Fossil, and every check-in now comes with identifying information that can trace it back to its original source.  You can use the SQLITE_SOURCE_ID #define or the sqlite3_sourceid() C/C++ interface or the sqlite_source_id() SQL function to access this information.  These will give you the date and time of the check-in and the SHA1 hash of the entire check-in.  This gives you complete traceability of the specific check-in of SQLite that you are using and largely erases the need to stick to an "official release".

We can give you guidance about which SQLite check-ins are suitable for use in Mozilla Central and which you should avoid, if you want to go with this approach.

	Shawn Wilsher :sdwilsh
	Comment 14 • 15 years ago

The problem with that approach is that it will break linux distros that ship with system SQLite.

	D. Richard Hipp
	Comment 15 • 15 years ago

I've argued that such linux distros are already broken, though I've found that argument doesn't cut much hay with the distros themselves, so let's move on....

Maybe you want to put a intermediate check-in of SQLite in nightly builds, then settle on an official SQLite release version when you get close to a FF release?  That gives you the opportunity to run with an SQLite that contains all of the latest bug fixes and thereby avoid unsightly work-arounds in the FF code.

	Shawn Wilsher :sdwilsh
	Comment 16 • 15 years ago

We should just wait.  I don't actually have time to get a new version landing for a for weeks anyway, so you might have released by then!

	Shawn Wilsher :sdwilsh
	Comment 17 • 15 years ago

I'll backout bug 519270 once the tree reopens today.

	Shawn Wilsher :sdwilsh
	Comment 18 • 15 years ago

Fixed by backout of bug 519270.