Closed Bug 1378033 Opened 5 years ago Closed 5 years ago

JavaScript error: nsUpdateTimerManager.js, line 311: TypeError: invalid 'in' operand this._timers

Categories

(Toolkit :: Application Update, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla56
Tracking Status
firefox56 --- fixed

People

(Reporter: jaws, Assigned: robert.strong.bugs)

References

Details

Attachments

(1 file, 1 obsolete file)

I got this JavaScript error when running a debug build,
http://searchfox.org/mozilla-central/rev/a3a739de04ee6134c11546568a33dbb6a6a29907/toolkit/components/timermanager/nsUpdateTimerManager.js#311

>  /**
>   * See nsIUpdateTimerManager.idl
>   */
>  registerTimer: function TM_registerTimer(id, callback, interval) {
>    LOG("TimerManager:registerTimer - id: " + id);
>    if (id in this._timers && callback != this._timers[id].callback) { /* <-------- this line */
>      LOG("TimerManager:registerTimer - Ignoring second registration for " + id);
>      return;
>    }
>    let prefLastUpdate = PREF_APP_UPDATE_LASTUPDATETIME_FMT.replace(/%ID%/, id);
>    // Initialize the last update time to 0 when the preference isn't set so
>    // the timer will be notified soon after a new profile's first use.
>    let lastUpdateTime = getPref("getIntPref", prefLastUpdate, 0);
>    let now = Math.round(Date.now() / 1000);
>    if (lastUpdateTime > now) {
>      lastUpdateTime = 0;
>    }
>    if (lastUpdateTime == 0) {
>      Services.prefs.setIntPref(prefLastUpdate, lastUpdateTime);
>    }
>    this._timers[id] = {callback,
>                        interval,
>                        lastUpdateTime};
>
>    this._ensureTimer(interval * 1000);
>  },
>
>  unregisterTimer: function TM_unregisterTimer(id) {
>    LOG(`TimerManager:unregisterTimer - id: ${id}`);
>    if (id in this._timers) {
>      delete this._timers[id];
>    } else {
>      LOG(`TimerManager:registerTimer - Ignoring unregistration request for unknown id: ${id}`);
>    }
>  },
>
>  classID: Components.ID("{B322A5C0-A419-484E-96BA-D7182163899F}"),
>  QueryInterface: XPCOMUtils.generateQI([Ci.nsIUpdateTimerManager,
>                                         Ci.nsITimerCallback,
>                                         Ci.nsIObserver])
>};
I see this too, on opt builds. It looks from the code like something is calling registerTimer after profile-before-change has already fired. I guess ideally we should both (a) fix the consumer not to do that (ie this might be a bigger bug than just the logspam, if whatever it's trying to schedule is actually important or if it's still trying to do initialization even though the process is dying) and (b) fix registerTimer to throw a more descriptive error if it gets called when _timers is already null.
This is likely due to in part bug 1375077 which shuts nsUpdatetimerManager down earlier than we used to and it appears that when running tests profile-before-change is sent before the application is shutting down. Option b is a no brainer and I don't think option a will provide any value in the non test world where all notifications are sent... at worst the component fails after it has already shut down.
Note: the registration id is telemetry_modules_ping
Attached patch patch rev1 (obsolete) — Splinter Review
Assignee: nobody → robert.strong.bugs
Status: NEW → ASSIGNED
Attachment #8883757 - Flags: review?(mhowell)
chutten, just an fyi that telemetry's registerTimer call is reporting an error in tests due to it delaying until after profile-before-change is called. See comment #4 for more details.
Flags: needinfo?(chutten)
Attached patch patch rev2Splinter Review
While I'm here I switched the inconsistent though valid logging style that was recently added as well as fixed a typo in the log message.
Attachment #8883757 - Attachment is obsolete: true
Attachment #8883757 - Flags: review?(mhowell)
Attachment #8883762 - Flags: review?(mhowell)
Attachment #8883762 - Flags: review?(mhowell) → review+
Pushed by rstrong@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/39e61f5e8fc5
Don't try to register timers during shutdown in nsUpdateTimerManager.js. r=mhowell
https://hg.mozilla.org/mozilla-central/rev/39e61f5e8fc5
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla56
Good to know. +ni Dexter, gfritzsche for visibility.
Flags: needinfo?(gfritzsche)
Flags: needinfo?(chutten)
Flags: needinfo?(alessio.placitelli)
Thanks.
For this specific call, this seems non-critical.
Unless there is specific impact this seems fine to leave as is (with the error being handled in the update timer manager now).
Flags: needinfo?(gfritzsche)
+ Marco (FYI)
Flags: needinfo?(alessio.placitelli)
You need to log in before you can comment on or make changes to this bug.