834995 - Possible race condition in mozStorageConnection.cpp

Status: RESOLVED FIXED

(Toolkit :: Storage, defect)

Description

[Gregor Wagner [:gwagner]]

I see some memory corruption going on with mFunctions in mozStorageConnection.cpp.
All the functions that touch mFunctions use SQLiteMutexAutoLock lockedScope(sharedDBMutex); execept Connection::Clone.
Do we miss a lock here?

(gdb) bt
#0  0x41b10976 in SearchTable (table=0x48f8df68, key=0x48d7fd30, keyHash=2323735344, op=PL_DHASH_LOOKUP)
    at /Volumes/2mac/gaia/b2g18/unagibuild/xpcom/build/pldhash.cpp:394
#1  0x41b10cec in PL_DHashTableOperate (table=0x48f8df68, key=0x48d7fd30, op=PL_DHASH_LOOKUP)
    at /Volumes/2mac/gaia/b2g18/unagibuild/xpcom/build/pldhash.cpp:587
#2  0x40a3864c in nsTHashtable<nsBaseHashtableET<nsCStringHashKey, unsigned int> >::GetEntry (this=0x48f8df68, aKey=...)
    at ../../../dist/include/nsTHashtable.h:148
#3  0x4171e736 in nsBaseHashtable<nsCStringHashKey, mozilla::storage::Connection::FunctionInfo, mozilla::storage::Connection::FunctionInfo>::Get (this=0x48f8df68, aKey=..., pData=0x0) at ../../dist/include/nsBaseHashtable.h:104
#4  0x4171df9c in mozilla::storage::Connection::RemoveFunction (this=0x48f8df40, aFunctionName=...)
    at /Volumes/2mac/gaia/b2g18/storage/src/mozStorageConnection.cpp:1344
#5  0x41235d30 in mozilla::dom::indexedDB::CommitHelper::Run (this=0x4c9f5980)
    at /Volumes/2mac/gaia/b2g18/dom/indexedDB/IDBTransaction.cpp:948
#6  0x4124a2e4 in mozilla::dom::indexedDB::TransactionThreadPool::TransactionQueue::Run (this=0x48f8dee0)
    at /Volumes/2mac/gaia/b2g18/dom/indexedDB/TransactionThreadPool.cpp:639
#7  0x41b5796a in nsThreadPool::Run (this=0x4856d740) at /Volumes/2mac/gaia/b2g18/xpcom/threads/nsThreadPool.cpp:187
#8  0x41b5589c in nsThread::ProcessNextEvent (this=0x48fcce20, mayWait=true, result=0x48d7fe97)
    at /Volumes/2mac/gaia/b2g18/xpcom/threads/nsThread.cpp:620
#9  0x41b0f592 in NS_ProcessNextEvent_P (thread=0x48fcce20, mayWait=true)
    at /Volumes/2mac/gaia/b2g18/unagibuild/xpcom/build/nsThreadUtils.cpp:237
#10 0x41b54cbe in nsThread::ThreadFunc (arg=0x48fcce20) at /Volumes/2mac/gaia/b2g18/xpcom/threads/nsThread.cpp:258
#11 0x40392254 in _pt_root (arg=0x4c3a7f90) at /Volumes/2mac/gaia/b2g18/nsprpub/pr/src/pthreads/ptthread.c:156
#12 0x40095e18 in __thread_entry (func=0x4039219d <_pt_root>, arg=0x4c3a7f90, tls=<value optimized out>)
    at bionic/libc/bionic/pthread.c:217
#13 0x4009596c in pthread_create (thread_out=<value optimized out>, attr=0xbef29158, start_routine=0x4039219d <_pt_root>, 
    arg=0x4c3a7f90) at bionic/libc/bionic/pthread.c:357
#14 0x00000000 in ?? ()

Comment 1

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 2

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

Comment on attachment 706716 [details] [diff] [review]
patch

Review of attachment 706716 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, mFunctions should be protected by sharedDBMutex.
Thanks!

Comment 3

[Gregor Wagner [:gwagner]]

I found this bug during debugging bug 832385. Maybe they are related.

Comment 4

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1bd83d06914

Comment 5

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Nice catch.

Comment 6

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/c1bd83d06914

Comment 7

[Gregor Wagner [:gwagner]]

Comment on attachment 706716 [details] [diff] [review]
patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): na
User impact if declined: random memory corruption / crashes
Testing completed (on m-c, etc.): on mc
Risk to taking this patch (and alternatives if risky): minor
String or UUID changes made by this patch: na

Comment 8

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/325a8530f92e

Comment 9

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_0_0/rev/1f22eda11281

Comment 10

[Alex Keybl [:akeybl]]

Comment on attachment 706716 [details] [diff] [review]
patch

Not critical enough to fix for FF19, but I see no reason to prevent uplift to FF20 given the risk evaluation.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/66a7e8d34d83