Closed Bug 714202 Opened 11 years ago Closed 11 years ago

Addon Sync: addons failed: this._changes[0] is undefined in pruneChangesBeforeDate


(Firefox :: Sync, defect)

Not set



Tracking Status
firefox11 --- verified


(Reporter: i, Assigned: gps)



(3 files, 1 obsolete file)

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:12.0a1) Gecko/20111229 Firefox/12.0a1
Build ID: 20111229031056

Steps to reproduce:

I enabled add-on sync. 

Actual results:

When I updated nightly at Dec 29th, I encountered an syncing error. When I disable add-on sync, no error occur. I have to reset the recovery key to solve it.
Severity: normal → blocker
OS: All → Windows 7
Priority: -- → P1
Hardware: All → x86_64
Greg, take a look?
Assignee: nobody → gps
Severity: blocker → normal
Ever confirmed: true
Priority: P1 → --
Summary: sync error when add-on sync enabled → Addon Sync: addons failed: this._changes[0] is undefined in pruneChangesBeforeDate
It appears to be failing in the 2nd line of the following:

    while (this._changes.length > 0) {
      if (this._changes[0][0] >= date) {

      delete this._changes[0];

The elements of this._changes should always be arrays. This is most weird. The next step is to figure out how an undefined element is being written to that array.

As part of fixing this, I'll have to determine whether it is possible bad data was being serialized to the reconciler's JSON file. If so, I'll need to sanitize data on load so issues in the wild repair themselves.

Jian: can you upload or give a link to a copy of your weave/addonsreconciler.json file from the profile directory? This file may contain personal information. If you don't want the world to see it, you can email it to me at
Looks like we are using the delete operator incorrectly.


> When you delete an array element, the array length is not affected. For
> example, if you delete a[3], a[4] is still a[4] and a[3] is undefined.


It also looks like there is no explicit test coverage for this. I'll code up a fix.

I don't think undefined is being written to the reconciler's JSON file because JS will error before the mutated array has an opportunity to be written. But, I would still like verification from OP that the saved JSON is sane.
I switched the implementation to use Array.filter. Includes a new test ensuring that the function behaves properly. The test failed on previous code and passes with this patch.

This patch will eventually need to land in Firefox 11 (currently Aurora) because it is a critical Sync bug.
Attachment #585010 - Flags: review?(rnewman)
OS: Windows 7 → All
Hardware: x86_64 → All
Comment on attachment 585010 [details] [diff] [review]
Use Array.filter to produce new array

Review of attachment 585010 [details] [diff] [review]:

::: services/sync/tests/unit/test_addons_reconciler.js
@@ +174,5 @@
> +  do_check_eq(young, reconciler._changes[0][0]);
> +  do_check_eq("bar", reconciler._changes[0][2]);
> +
> +  run_next_test();
> +});

Please also add tests for:

* Pruning an empty change array
* Pruning a change array for which no items will be removed
* Pruning a change array for which all items will be removed
Attachment #585010 - Flags: review?(rnewman)
Attachment #585010 - Flags: review+
Attachment #585010 - Flags: approval-mozilla-aurora?
Patch with MOAR tests per review. All tests pass.
Attachment #585010 - Attachment is obsolete: true
Attachment #585010 - Flags: approval-mozilla-aurora?
Attachment #585027 - Flags: review?(rnewman)
Attachment #585027 - Flags: review?(rnewman)
Attachment #585027 - Flags: review+
Attachment #585027 - Flags: approval-mozilla-aurora?
This is the addonsreconciler.json in my profile directory. I think there's no particular personal information in it. Hope it will help you to fix the bug.
Pushed to inbound:

rnewman gave approval in IRC to skip s-c and get this in m-c as quick as possible.
Jian: I wanted to thank you for an awesome first bug report! It's pretty rare (at least for Sync) for someone's first bug report to be completely diagnosable from the initial comment. Your thoroughness is very much appreciated. Have a great new year.
It's my pleasure. I'm glad to hear that.

I love FF though it's not perfect. Actually I'm majored in Computer Architecture and I once planed to read the source code of FF, but I feel that it is really difficult to begin with. Anyway, I've made a little contribution to FF now, which makes me very happy. lol

Happy new year!
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla12
Comment on attachment 585027 [details] [diff] [review]
Use Array.filter to produce new array, v2

[Triage Comment]
Approved for Aurora to fix a major add-on sync issue.
Attachment #585027 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
I don't see this just enabling add-ons sync on nightly or aurora.  Is there any specific setup or conditions to reproduce this?
Tracy: this is fixed in Nightly and will not repro there. To repro on Aurora 11.0a2, if you perform enough add-on actions, you should eventually run into this bug.
Whiteboard: [qa+]
Whiteboard: [qa+]
Component: Firefox Sync: Backend → Sync
Product: Cloud Services → Firefox
You need to log in before you can comment on or make changes to this bug.