Transaction success/failure events can double fire on settings

RESOLVED FIXED in Firefox 34, Firefox OS v2.1


Firefox OS
3 years ago
3 years ago


(Reporter: qdot, Assigned: qdot)



2.1 S5 (26sep)
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:2.1+, firefox33 wontfix, firefox34 fixed, firefox35 fixed, b2g-v1.4 unaffected, b2g-v2.0 unaffected, b2g-v2.0M unaffected, b2g-v2.1 fixed, b2g-v2.2 fixed)


(Whiteboard: [systemsfe])


(1 attachment)

Due to the clean-up functions that slid in at the end of 2.1 FL, we can now have situations where onsettingstransactionsuccess/failure fires twice for the same transaction: once when it finishes, then once when the SettingsManager dies and tries to clean it up. This usually happens when a transaction fails due to a permissions error, but doesn't unregister itself from the SettingsManager in the child process.
[Blocking Requested - why for this release]: Bug in new Settings API that landed as part of FL. May cause undefined behavior if user callbacks fire twice.
blocking-b2g: --- → 2.1?
blocking-b2g: 2.1? → 2.1+
Whiteboard: [systemsfe]
Target Milestone: --- → 2.1 S4 (12sep)
Keywords: regression
Created attachment 8488777 [details] [diff] [review]
Patch 1 (v1) - Make sure Settings API only calls finalize once

Put checks in SettingsManager and fixed checked in SettingsRequestManager to make sure we can't call finalize multiple times. Also fixed mochitest bandaid that was actually wrong in the first place.
Attachment #8488777 - Flags: review?(bent.mozilla)
status-b2g-v1.4: --- → unaffected
status-b2g-v2.0: --- → unaffected
status-b2g-v2.0M: --- → unaffected
status-b2g-v2.1: --- → affected
status-b2g-v2.2: --- → affected
Attachment #8488777 - Flags: review?(bent.mozilla) → review+
Comment on attachment 8488777 [details] [diff] [review]
Patch 1 (v1) - Make sure Settings API only calls finalize once

Approval Request Comment
[Feature/regressing bug #]: Bug 900551
[User impact if declined]: Transaction events fire incorrectly for settings
[Describe test coverage new/current, TBPL]: mochitest coverage
[Risks and why]: None
[String/UUID change made/needed]: None
Attachment #8488777 - Flags: approval-mozilla-aurora?
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)


3 years ago
status-b2g-v2.2: affected → fixed


3 years ago
Attachment #8488777 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
status-b2g-v2.1: affected → fixed
status-firefox33: --- → wontfix
status-firefox34: --- → fixed
status-firefox35: --- → fixed
Backed out for Mnw permafail.
status-b2g-v2.1: fixed → affected
status-firefox34: fixed → affected
Flags: needinfo?(kyle)
Keywords: branch-patch-needed
This has to land after bug 1058158, otherwise it'll permafail Mnw due to the system reset.
Depends on: 1058158
Flags: needinfo?(kyle)
status-b2g-v2.1: affected → fixed
status-firefox34: affected → fixed
Keywords: branch-patch-needed
Blocks: 1107982
You need to log in before you can comment on or make changes to this bug.