Closed Bug 718455 (SQLite3.7.10) Opened 12 years ago Closed 12 years ago

Upgrade to SQLite 3.7.10

Categories

(Toolkit :: Storage, defect)

Product:
Toolkit
Component:
Storage
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla12

People

(Reporter: RyanVM, Assigned: RyanVM)

References

Details

Attachments

(2 files)

Upgrade to SQLite 3.7.10 - SQLite changes
1.28 MB, patch
bent.mozilla
: feedback+
Details | Diff | Splinter Review
Upgrade to SQLite 3.7.10 - Mozilla changes
2.44 KB, patch
mak
: review+
Details | Diff | Splinter Review 
+++ This bug was initially created as a clone of Bug #710183 +++

This is mostly for bugfixes, stability and some qyuery planner optimization. Additionally, 3.7.10 adds support for "PRAGMA shrink_memory" and the new sqlite3_db_filename() interface:

http://sqlite.org/releaselog/3_7_8.html
http://sqlite.org/releaselog/3_7_9.html
http://sqlite.org/releaselog/3_7_10.html

This should also remove the need for the fsync workaround we used on 3.7.7.1 for Android.
I'll spin up patches and Tryserver builds for this.
Assignee: nobody → ryanvm
Alias: SQLite3.7.10
Try run for 991b1ea41b08 is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=991b1ea41b08
Results (out of 80 total builds):
    exception: 5
    success: 41
    warnings: 19
    failure: 15
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ryanvm@gmail.com-991b1ea41b08
Yeah, this still torches MSVC 2010 PGO builds. Looks like it's OK on non-PGO builds (that email will be coming later).
Looks like Win64 has issues, though.

Running transaction helper tests...
TEST-PASS | e:/builds/moz2_slave/try-w64/build/storage/test/test_transaction_helper.cpp
27 of 27 tests passed
Finished running transaction helper tests.
Running statement scoper tests...
TEST-PASS | e:/builds/moz2_slave/try-w64/build/storage/test/test_statement_scoper.cpp
12 of 12 tests passed
Finished running statement scoper tests.
make[2]: Leaving directory `/e/builds/moz2_slave/try-w64/build/obj-firefox/storage/test'
make[1]: Leaving directory `/e/builds/moz2_slave/try-w64/build/obj-firefox/storage'
make[2]: *** [check] Error 116
make[1]: *** [check] Error 2
make: *** [check] Error 2
Ben, can you take a look at this? I'm wondering if the test_quota.c change is what's causing the Win64 test failure.
Attachment #589085 - Flags: feedback?(bent.mozilla)
(In reply to Ryan VanderMeulen from comment #4)
> Looks like Win64 has issues, though.
> 
> Running transaction helper tests...
> TEST-PASS |
> e:/builds/moz2_slave/try-w64/build/storage/test/test_transaction_helper.cpp
> 27 of 27 tests passed
> Finished running transaction helper tests.
> Running statement scoper tests...
> TEST-PASS |
> e:/builds/moz2_slave/try-w64/build/storage/test/test_statement_scoper.cpp
> 12 of 12 tests passed
> Finished running statement scoper tests.
> make[2]: Leaving directory
> `/e/builds/moz2_slave/try-w64/build/obj-firefox/storage/test'
> make[1]: Leaving directory
> `/e/builds/moz2_slave/try-w64/build/obj-firefox/storage'
> make[2]: *** [check] Error 116
> make[1]: *** [check] Error 2
> make: *** [check] Error 2

It appears that the SQLiteMutex tests are the next to run after the scoper tests.
Try run for 3127a2c910bd is complete.
Detailed breakdown of the results available here:
    https://tbpl.mozilla.org/?tree=Try&rev=3127a2c910bd
Results (out of 309 total builds):
    exception: 2
    success: 254
    warnings: 48
    failure: 5
Builds (or logs if builds failed) available at:
http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/ryanvm@gmail.com-3127a2c910bd
 Timed out after 06 hours without completing.
we may consider disabling our custom allocator on Win till we move to msvc2010, may we check if doing that would solve the PGO issue?
http://mxr.mozilla.org/mozilla-central/source/storage/src/mozStorageService.cpp#516

Not sure about the win64 problem, Error 116 is a bit generic...
Whiteboard: [MemShrink]
Ryan, are you available for the check in comment 9? were you able to reproduce the test failure on win64, do you have a win64 box?
So, win64 builders stop at test_mutex.exe. Doing a make check locally on my win64 box (but building a win32), I hit a _CrtIsValidHeapPointer assertion with this stack:

   msvcr100d.dll!__msize_dbg()  + 0x13c bytes   
    msvcr100d.dll!__msize()  + 0x10 bytes   
    mozsqlite3.dll!sqlite3MemSize(void * pPrior)  Line 15262 + 0x10 bytes    C
    mozsqlite3.dll!sqlite3MallocSize(void * p)  Line 18961 + 0xa bytes    C
    mozsqlite3.dll!mallocWithAlarm(int n, void * * pp)  Line 18797 + 0x9 bytes    C
    mozsqlite3.dll!sqlite3Malloc(int n)  Line 18822 + 0xd bytes    C
    mozsqlite3.dll!sqlite3MallocZero(int n)  Line 19086 + 0x9 bytes    C
    mozsqlite3.dll!winMutexAlloc(int iType)  Line 18370 + 0x7 bytes    C
    mozsqlite3.dll!sqlite3MutexAlloc(int id)  Line 17280 + 0xa bytes    C
    mozsqlite3.dll!sqlite3_initialize()  Line 46810 + 0x7 bytes    C
    mozsqlite3.dll!sqlite3_mutex_alloc(int id)  Line 17270 + 0x5 bytes    C
    test_mutex.exe!test_AutoLock()  Line 62 + 0xd bytes    C++
    test_mutex.exe!main(int aArgc, char * * aArgv)  Line 55 + 0xc bytes    C++
    test_mutex.exe!__tmainCRTStartup()  Line 555 + 0x19 bytes    C
    test_mutex.exe!mainCRTStartup()  Line 371    C
    kernel32.dll!@BaseThreadInitThunk@12()  + 0x12 bytes   
    ntdll.dll!___RtlUserThreadStart@8()  + 0x27 bytes   
    ntdll.dll!__RtlUserThreadStart@8()  + 0x1b bytes
and locally this change to our sqlite Makefile.in fixes the assertion:

ifeq ($(OS_ARCH),WINNT)
ifdef MOZ_MEMORY
DEFINES += -DHAVE_MALLOC_USABLE_SIZE
endif
endif
Comment on attachment 589085 [details] [diff] [review]
Upgrade to SQLite 3.7.10 - SQLite changes

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

As long as everything compiles and all the tests pass I say go for it!
Attachment #589085 - Flags: feedback?(bent.mozilla) → feedback+
Comment on attachment 589086 [details] [diff] [review]
Upgrade to SQLite 3.7.10 - Mozilla changes

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

these boilerplate changes are fine, we'll coalesce further needed changes in other patches.
Attachment #589086 - Flags: review?(mak77) → review+
So, try confirmed that my change in comment 13, forcing to use our own malloc_usable_size, rather than Windows _msize, solves the win64 issue.

The remaining problem are the Windows PGO crashes with this stack:

 0  mozglue.dll!je_free [jemalloc.c:e5ea4b87f037 : 6580 + 0xb]
    eip = 0x72463481   esp = 0x002ec1dc   ebp = 0x00000000   ebx = 0x00000001
    esi = 0x0084e720   edi = 0x008aa690   eax = 0x00000000   ecx = 0x00000001
    edx = 0x6df92424   efl = 0x00010246
    Found by: given as instruction pointer in context
 1  mozsqlite3.dll!sqlite3_bind_text + 0xc5
    eip = 0x6df4983c   esp = 0x002ec1f4   ebp = 0x002ec224
    Found by: call frame info with scanning
 2  xul.dll!mozilla::storage::`anonymous namespace'::variantToSQLiteT<mozilla::storage::`anonymous namespace'::BindingColumnData>(mozilla::storage::A0xe7c8ad06::BindingColumnData,nsIVariant *) [variantToSQLiteT_impl.h:e5ea4b87f037 : 109 + 0x1b]
    eip = 0x6c4aa903   esp = 0x002ec22c   ebp = 0x002ec340
    Found by: previous frame's frame pointer
 3  xul.dll!mozilla::storage::BindingParams::bind(sqlite3_stmt *) [mozStorageBindingParams.cpp:e5ea4b87f037 : 254 + 0x10]
    eip = 0x6c4aa7cf   esp = 0x002ec348   ebp = 0x00000010
    Found by: previous frame's frame pointer
 4  xul.dll!mozilla::storage::Statement::ExecuteStep(bool *) [mozStorageStatement.cpp:e5ea4b87f037 : 606 + 0x13]
    eip = 0x6c487391   esp = 0x002ec360   ebp = 0x002ec388
    Found by: call frame info with scanning
 5  xul.dll!nsPermissionManager::UpdateDB(nsPermissionManager::OperationType,mozIStorageStatement *,__int64,nsACString_internal const &,nsACString_internal const &,unsigned int,unsigned int,__int64) [nsPermissionManager.cpp:e5ea4b87f037 : 1128 + 0xc]
    eip = 0x6c55e0ff   esp = 0x002ec37c   ebp = 0x002ec388
    Found by: call frame info with scanning
 6  xul.dll!nsPermissionManager::AddInternal(nsCString const &,nsCString const &,unsigned int,__int64,unsigned int,__int64,nsPermissionManager::NotifyOperationType,nsPermissionManager::DBOperationType) [nsPermissionManager.cpp:e5ea4b87f037 : 530 + 0x27]
    eip = 0x6c38b6f3   esp = 0x002ec390   ebp = 0x002ec410
    Found by: previous frame's frame pointer
 7  xul.dll!nsPermissionManager::Add(nsIURI *,char const *,unsigned int,unsigned int,__int64) [nsPermissionManager.cpp:e5ea4b87f037 : 432 + 0x29]
    eip = 0x6c564c88   esp = 0x002ec418   ebp = 0x002ec4bc
    Found by: previous frame's frame pointer
 8  xul.dll!NS_InvokeByIndex_P [xptcinvoke.cpp:e5ea4b87f037 : 102 + 0x2]
    eip = 0x6c5d611f   esp = 0x002ec4c4   ebp = 0x002ec4f0
    Found by: previous frame's frame pointer
 9  xul.dll!XPCWrappedNative::CallMethod(XPCCallContext &,XPCWrappedNative::CallMode) [XPCWrappedNative.cpp:e5ea4b87f037 : 2196 + 0x27d]
    eip = 0x6c44e3df   esp = 0x002ec4f8   ebp = 0x002ec71c
    Found by: call frame info
So, good news, building with -O2 instead of -O3 does not crash. So I have a workaround for both issues, will file bugs and attach patchs.
I was waiting Win PGO but with today's traffic it's taking forever.

https://hg.mozilla.org/integration/mozilla-inbound/rev/0acc07243349
https://hg.mozilla.org/integration/mozilla-inbound/rev/92b6a058907f
Status: NEW → ASSIGNED
Flags: in-testsuite-
Target Milestone: --- → mozilla12
https://hg.mozilla.org/mozilla-central/rev/0acc07243349
https://hg.mozilla.org/mozilla-central/rev/92b6a058907f
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
No longer depends on: 722188
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: