Closed Bug 945813 Opened 8 years ago Closed 8 years ago

Paper over cycle collection problem in IndexedDB

Categories

(Core :: Storage: IndexedDB, defect)

x86_64
Linux
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla28

People

(Reporter: wingo, Assigned: wingo)

References

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

Bug 943409 describes an instance of a cycle collection problem in indexedDB, and adds a minimal test case.  There are three instances of this problem in the codebase:

  dom/indexedDB/test/unit/test_cursor_mutation.js
  dom/indexedDB/test/unit/test_index_object_cursors.js
  dom/indexedDB/test/unit/test_index_update_delete.js

The attached patch works around the issues in these three tests, in preparation for bug 927782.
Assignee: nobody → wingo
Comment on attachment 8341801 [details] [diff] [review]
Paper over cycle collection problem in IndexedDB

Adding mccr8 for a quick review; it's the same as bug 943409, but with two additional tests.  Adding khuey as suggested there for rubber stampage.
Attachment #8341801 - Flags: review?(khuey)
Attachment #8341801 - Flags: review?(continuation)
Attachment #8341801 - Flags: review?(continuation) → review+
Keywords: checkin-needed
Whoops, meant to back this one out the first time for the xpcshell crashes but accidentally did bug 945828 instead. Please make sure this passes on Try before requesting checkin again.
https://hg.mozilla.org/integration/mozilla-inbound/rev/cb35692e00ec
Yep, sorry about that.  Odd that it passes locally...
Attachment #8341801 - Attachment is obsolete: true
Attachment #8342346 - Attachment is obsolete: true
Comment on attachment 8342347 [details] [diff] [review]
Paper over cycle collection problem in IndexedDB   khuey

New patch uses eval('') to force all locals onto the scope chain, to better simulate what will happen after bug 927782 lands.  Instead of relying on gc, manually null out locals that cause cycles.  Tested locally on linux64-debug; trybuild here: https://tbpl.mozilla.org/?tree=Try&rev=a76ba05fd069

Marking r+ as it is pretty much the same as previous patches.
Attachment #8342347 - Flags: review+
Looking green, https://tbpl.mozilla.org/?tree=Try&rev=fc1f986c5aac -- let's hit it
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/815cd189ae2c
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla28
Depends on: 951483
Whiteboard: [qa-]
You need to log in before you can comment on or make changes to this bug.