Closed Bug 1061805 Opened 5 years ago Closed 5 years ago

Transaction success/failure events can double fire on settings

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

x86_64
Linux
defect
Not set

Tracking

(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)

RESOLVED FIXED
2.1 S5 (26sep)
blocking-b2g 2.1+
Tracking Status
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

People

(Reporter: qdot, Assigned: qdot)

References

Details

(Keywords: regression, Whiteboard: [systemsfe])

Attachments

(1 file)

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)
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)
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?
https://hg.mozilla.org/mozilla-central/rev/0d36703e1a75
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: 2.1 S4 (12sep) → 2.1 S5 (26sep)
Attachment #8488777 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
This has to land after bug 1058158, otherwise it'll permafail Mnw due to the system reset.
Depends on: 1058158
Flags: needinfo?(kyle)
You need to log in before you can comment on or make changes to this bug.