507790 - Sqlite could execute async queries in the middle of UNIONED sync queries

Status: RESOLVED WONTFIX

(Toolkit :: Storage, defect)

Description

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Is it possible that Sqlite can execute an async query in the middle of another sync query, for example if the sync query is an UNION ALL query.
This means underlying data can change while a query is running, and that can potentially cause returning wrong/duplicated results (this is imho a bad experience, while a query is running data should be guaranteed to be fixed).

Solution should involve using only sync or only async queries, and sounds like for Sqlite folks this is correct behavior.
I'm filing this bug to track random failures and bugs that depend on this behavior.

Comment 1

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

well actually i see something even more strange, sqlite is giving me a "p->inTrans->Trans_none" assertion that comes from nsNavBookmarks::QueryFolderChildren.

That indicates some bad and strange transaction issue (transaction gone in the middle?)

Replacing UNION ALL with UNION fixes the assertion, sounds like a transaction is being terminated between the 2 queries, and the query ends up with a wrong transactions count.

Comment 2

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

for "the 2 queries" i mean the 2 parts of the mDBGetChildren query that are UNIONed with UNION ALL.

Comment 3

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

 	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++
 	cccccccc()

Comment 4

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

and i can confirm replacing UNION ALL with UNION in that query solves everything, so there is definately some threading issue with UNION ALL, and probably transactions.

Comment 5

[Shawn Wilsher :sdwilsh]

That's going to be a different bug than you originally filed.  Please file a new one.

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

you are right, i wanted to show you this before filing a new bug. Btw, filed Bug 520445

Comment 7

[Dietrich Ayala (:dietrich)]

The only bug this depends on is marked WONTFIX. What else needs to happen here? Go 100% async? If so, can you mark those bugs as blocking this?

Comment 8

[Shawn Wilsher :sdwilsh]

(In reply to comment #7)
> The only bug this depends on is marked WONTFIX. What else needs to happen here?
> Go 100% async? If so, can you mark those bugs as blocking this?
Yeah, we really need to be 100% async.

Comment 9

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

Shawn, I think this bug right now is not much useful. We cannot fix the underlying sqlite problem because it's how it works, and we cannot workaround it as well since it could cause some sort of thread-locking.
All oranges are now fixed thanks to Places branch, so I'd suggest to WONTFIX this.

Comment 10

[Shawn Wilsher :sdwilsh]

Sure