Transaction success/failure events can double fire on settings

RESOLVED FIXED in Firefox 34, Firefox OS v2.1

Status

Firefox OS
Gaia::Settings
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: qdot, Assigned: qdot)

Tracking

({regression})

unspecified
2.1 S5 (26sep)
x86_64
Linux
regression
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)

Details

(Whiteboard: [systemsfe])

Attachments

(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+
https://hg.mozilla.org/integration/b2g-inbound/rev/0d36703e1a75
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?
https://hg.mozilla.org/mozilla-central/rev/0d36703e1a75
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)

Updated

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

Updated

3 years ago
Attachment #8488777 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/25e5b5838db2
status-b2g-v2.1: affected → fixed
status-firefox33: --- → wontfix
status-firefox34: --- → fixed
status-firefox35: --- → fixed
Backed out for Mnw permafail.
https://hg.mozilla.org/releases/mozilla-aurora/rev/fe591acbe83b

https://tbpl.mozilla.org/php/getParsedLog.php?id=48207813&tree=Mozilla-Aurora
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)
https://hg.mozilla.org/releases/mozilla-aurora/rev/1889babe666c
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.