Bug 1328161 (SQLite3.16.2)

Upgrade to SQLite 3.16.2

RESOLVED FIXED in Firefox 53

Status

()

defect
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: ionnv, Assigned: RyanVM)

Tracking

unspecified
mozilla53
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox53 fixed)

Details

()

Attachments

(1 attachment, 1 obsolete attachment)

https://sqlite.org/news.html

https://sqlite.org/releaselog/3_16_0.html

"SQLite version 3.16.0 is a regularly schedule maintenance release.

This release includes many microoptimizations that collectively reduce the CPU cycle count by about 9%, add there have been important enhancements to the command-line shell.

Support for PRAGMA functions is added, so that many pragma statements can be used as part of a larger SQL query. This is considered an experimental feature. We do not anticipate any changes to the PRAGMA function interface, but will keep continue to call this interface "experimental" for a few release cycles in case unforeseen issues arise.

See the change log for other enhancements."
Depends on: SQLite3.15.2
Assignee: nobody → ryanvm
FYI:  A bug was reported against SQLite 3.15.0 literally minutes after the 3.16.0 was pushed, and since the bug was previously unknown, it was not fixed by the 3.16.0 release.  We are working on 3.16.1 now.  Expect to see something within 24 hours.

On the other hand, the bug is in the new row-value logic added by SQLite 3.15.0, and specifically using row values in the SET clause of an UPDATE statement inside of a TRIGGER.  It is not something that Firefox is likely to run into.  So version 3.16.0 is safe to use.
Alias: SQLite3.16.0 → SQLite3.16.1
Summary: Upgrade to SQLite 3.16.0 → Upgrade to SQLite 3.16.1
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Try push has a real-looking IndexedDB failure:
https://treeherder.mozilla.org/logviewer.html#?job_id=66019555&repo=try
Perhaps the 3.16.1 changes related to trigger logic introduced a regression for the complicated trigger stuff IndexedDB does with externally stored blob reference counting?

The test in question http://searchfox.org/mozilla-central/source/dom/indexedDB/test/test_file_replace.html generates 100 blobs and in a single transaction, puts them successively to the same key, so that at the end only the blob with id === 100 will still be in the database when the transaction commits.  (The check on line 56 that does "id / 100 >> 0" is more clearly written as "Math.floor(id / 100)" and indicates that we expect the db refcount to be 0 for everything but id === 100.)

The reported error seems to indicate that the first blob (id === 1) didn't have its refcount decremented when the object_data_update_trigger fired when its references were obsoleted.

Relevant references (noting that file file is stupid huge):
- Triggers: http://searchfox.org/mozilla-central/source/dom/indexedDB/ActorsParent.cpp#1003
- Our update_refcount function implementation that uses: http://searchfox.org/mozilla-central/source/dom/indexedDB/ActorsParent.cpp#11392
(er, and of course :janv is most qualified to speak to the IndexedDB issue,  cc'ing)
Yeah, we do use triggers, so I would suggest to pay a close attention to this SQLite upgrade.
Is there an easy way for us SQLite developers to run this one test case using the nightly sources?
Ignore the previous.  I figured it out.  ("./mach mochitest dom/indexedDB/test/test_file_replace.html")
Yeah, but while you are there, it's better to run entire IDB test suite:
./mach mochitest dom/indexedDB
./mach xpcshell-test dom/indexedDB
./mach web-platform-tests IndexedDB
Thanks Jan.  I'll be adding that to our release checklist.

Meanwhile, I have managed to locate the offending SQLite change at http://www.sqlite.org/src/timeline?c=925840cf - we will work the problem on our end and report back.
Looking at the upstream commit history, it looks like the cause of the failures was found and fixed. I did a Try push off a recent snapshop of their 3.16 branch and it's looking good.
https://treeherder.mozilla.org/#/jobs?repo=try&revision=3d76006ad1279938a0a84dee7728dba0cfee1773
Alias: SQLite3.16.1 → SQLite3.16.2
Summary: Upgrade to SQLite 3.16.1 → Upgrade to SQLite 3.16.2
Thanks for the Try push!  We are in final testing for 3.16.2 now and expect to push the release later today.
There were no code changes between the Try push in comment 11 and the final release, so I think this is ready for review and landing.
Attachment #8823498 - Attachment is obsolete: true
Attachment #8824522 - Flags: review?(mak77)
Comment on attachment 8824522 [details] [diff] [review]
Upgrade SQLite to version 3.16.2

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

Thank you!
Attachment #8824522 - Flags: review?(mak77) → review+
https://hg.mozilla.org/mozilla-central/rev/ede3130bf486
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla53
Blocks: SQLite3.17.0
You need to log in before you can comment on or make changes to this bug.