Closed
Bug 1108328
(SQLite3.8.7.4)
Opened 9 years ago
Closed 9 years ago
Upgrade to SQLite 3.8.7.4
Categories
(Toolkit :: Storage, defect)
Toolkit
Storage
Tracking
()
RESOLVED
FIXED
mozilla37
People
(Reporter: ionnv, Assigned: RyanVM)
References
()
Details
Attachments
(1 file, 1 obsolete file)
9.63 KB,
patch
|
mak
:
review+
Sylvestre
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
http://www.sqlite.org/news.html "This release fixes two obscure bugs that can result in incorrect query results and/or application crashes, but not (as far as we can tell) security vulnerabilities. Both bugs have been latent in the code across multiple prior releases and have never before been encountered, so they are unlikely to cause problems. Nevertheless it seems prudent to publish fixes for them both. See the change log for details." http://www.sqlite.org/releaselog/3_8_7_3.html
Depends on: SQLite3.8.7.2
Assignee | ||
Comment 1•9 years ago
|
||
I think we're going to want this on Aurora as well.
Alias: SQLite3.8.7.3
Assignee: nobody → ryanvm
status-firefox36:
--- → affected
status-firefox37:
--- → affected
Assignee | ||
Comment 2•9 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=b18ef315ef23
Attachment #8533185 -
Flags: review?(mak77)
Assignee | ||
Updated•9 years ago
|
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Assignee | ||
Comment 3•9 years ago
|
||
This doesn't look good: https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3552418&repo=try
Assignee | ||
Comment 4•9 years ago
|
||
Comment on attachment 8533185 [details] [diff] [review] Upgrade to SQLite 3.8.7.3 Yeah, crashes aplenty with this update. https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3552349&repo=try https://treeherder.mozilla.org/ui/logviewer.html#?job_id=3553126&repo=try
Attachment #8533185 -
Flags: review?(mak77)
Comment 5•9 years ago
|
||
Looks like we omitted a necessary mutex on the 3.8.7.3 change. Can somebody please do a test run with www.sqlite.org/snapshot/sqlite-amalgamation-201412081745.zip to see if that clears the problem. (All of the existing SQLite test cases pass, so our own tests are of no help in determining whether or not the problem has been fixed.) The functional change in the snapshot above is: --- ../historical/3.8.7.3/sqlite3.c 2014-12-08 10:25:36.223141678 -0500 +++ sqlite3.c 2014-12-08 12:45:20.075284800 -0500 @@ -125896,11 +125896,13 @@ if( pDb->pBt ){ if( pDb->pSchema ){ /* Must clear the KeyInfo cache. See ticket [e4a18565a36884b00edf] */ + sqlite3BtreeEnter(pDb->pBt); for(i=sqliteHashFirst(&pDb->pSchema->idxHash); i; i=sqliteHashNext(i)){ Index *pIdx = sqliteHashData(i); sqlite3KeyInfoUnref(pIdx->pKeyInfo); pIdx->pKeyInfo = 0; } + sqlite3BtreeLeave(pDb->pBt); } sqlite3BtreeClose(pDb->pBt); pDb->pBt = 0; So you can just apply the patch above to the 3.8.7.3 SQLite rather than downloading a new amalgamation file, if that is easier. If the above change works for Firefox, then it will become SQLite version 3.8.7.4 later this week.
Assignee | ||
Comment 6•9 years ago
|
||
https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8cac5df2860e
Alias: SQLite3.8.7.3 → SQLite3.8.7.4
Summary: Upgrade to SQLite 3.8.7.3 → Upgrade to SQLite 3.8.7.4
Assignee | ||
Comment 7•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #6) > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8cac5df2860e No crashes like we were getting with 3.8.7.3. Better yet, Win7 talos appears to like it too!
Comment 8•9 years ago
|
||
A source-code audit, based on the prior failure, reveals another place where a mutex is needed, though apparently not a place that is important to FF as the tests are working without this extra mutex. There is a new snapshot at http://www.sqlite.org/snapshot/sqlite-amalgamation-201412082023.zip which contains both fixes, and also updates the version number to 3.8.7.4. Ryan, if you can put this new snapshot through the try server once, we'll release it as SQLite 3.8.7.4, probably early tomorrow morning EST. We'd probably be OK releasing it without the extra try-server run, but we want to avoid having to issue a version 3.8.7.5, so we'd feel better knowing that the new code works well in FF.
Assignee | ||
Comment 9•9 years ago
|
||
(In reply to D. Richard Hipp from comment #8) https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9362240c9b47 https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4e95b0760ebf (PGO enabled on supported platforms)
Assignee | ||
Comment 10•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #7) > (In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #6) > > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=8cac5df2860e > > No crashes like we were getting with 3.8.7.3. Better yet, Win7 talos appears > to like it too! However, I did manage to hit an instance of bug 1101419 still. https://bugzilla.mozilla.org/show_bug.cgi?id=1101419 We'll see what the build from comment 8 does.
Assignee | ||
Comment 11•9 years ago
|
||
(In reply to Ryan VanderMeulen [:RyanVM UTC-5] from comment #9) > (In reply to D. Richard Hipp from comment #8) > > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=9362240c9b47 > https://treeherder.mozilla.org/ui/#/jobs?repo=try&revision=4e95b0760ebf (PGO > enabled on supported platforms) I managed to hit both bug 1103966 and bug 1101419 in these runs, so while version 3.8.7.4 doesn't seem to make things any worse, it doesn't seem to have fixed anything either.
Comment 12•9 years ago
|
||
Yeah. The signatures of but 1103966 and bug 1101419 did not match the problems that 3.8.7.4 fixed. So it was a long shot. But worth trying.
Assignee | ||
Comment 13•9 years ago
|
||
Attachment #8533185 -
Attachment is obsolete: true
Attachment #8533903 -
Flags: review?(mak77)
Updated•9 years ago
|
Attachment #8533903 -
Flags: review?(mak77) → review+
Assignee | ||
Comment 14•9 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/cff44709cb11
Assignee | ||
Comment 15•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/cff44709cb11
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla37
Assignee | ||
Updated•9 years ago
|
Assignee | ||
Comment 16•9 years ago
|
||
Comment on attachment 8533903 [details] [diff] [review] Upgrade to SQLite 3.8.7.4 Approval Request Comment [Feature/regressing bug #]: Pre-existing SQLite bugs [User impact if declined]: Possible memory corruption and crashes [Describe test coverage new/current, TBPL]: On trunk and green Try runs and passes SQLite's test suites [Risks and why]: Low risk, we've already seen with the 3.8.7.3 patches in this bug that our test infra can detect problems caused by this code. [String/UUID change made/needed]: None
Attachment #8533903 -
Flags: approval-mozilla-aurora?
Updated•9 years ago
|
Attachment #8533903 -
Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Assignee | ||
Comment 17•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/b05e5b1cf94f
Comment 18•9 years ago
|
||
Looks like 3.8.8 just got released: http://www.sqlite.org/releaselog/3_8_8.html
Assignee | ||
Updated•9 years ago
|
Blocks: SQLite3.8.8.1
You need to log in
before you can comment on or make changes to this bug.
Description
•