Desktop Notifications: Race in nsDOMDesktopNotification constructor

RESOLVED FIXED in Firefox 18

Status

()

Core
DOM
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: gwagner, Assigned: mrbkap)

Tracking

unspecified
mozilla20
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(blocking-basecamp:-, firefox18 fixed, firefox19 fixed, firefox20 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

6 years ago
mrbkap found a race condition when we create the desktop notification object.
(Assignee)

Updated

6 years ago
Assignee: nobody → mrbkap
(Assignee)

Comment 2

6 years ago
Comment on attachment 682707 [details] [diff] [review]
patch

I'm not sure that this can actually race, but the early return in the if (!GetOwner()) case can definitely cause crashes. It's pretty hairy to reference count an object in its constructor, so this seems a lot saner in general.
Attachment #682707 - Flags: review?(doug.turner)

Updated

6 years ago
Attachment #682707 - Flags: review?(doug.turner) → review+
(Reporter)

Updated

6 years ago
blocking-basecamp: --- → ?
(Assignee)

Comment 4

6 years ago
Note to blocking-basecamp triagers: this is a very low-risk correctness bug that could prevent child-process crashes in certain edge cases.
https://hg.mozilla.org/mozilla-central/rev/9e2d2e78a956
Status: NEW → RESOLVED
Last Resolved: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
basecamp- but please nom for beta if this is really low risk.
blocking-basecamp: ? → -
(Assignee)

Comment 7

6 years ago
Comment on attachment 682707 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: In certain rare cases, content processes could crash when sending desktop notifications.
Testing completed (on m-c, etc.): This has been on m-c for a few days.

This patch is extremely low-risk, and is easily provably correct via code inspection.
Attachment #682707 - Flags: approval-mozilla-beta?

Comment 8

6 years ago
Comment on attachment 682707 [details] [diff] [review]
patch

[Triage Comment]
Very low risk fix that may resolve some B2G instability. Approving for branches.
Attachment #682707 - Flags: approval-mozilla-beta?
Attachment #682707 - Flags: approval-mozilla-beta+
Attachment #682707 - Flags: approval-mozilla-aurora+
https://hg.mozilla.org/releases/mozilla-aurora/rev/d75a9dd5b181
https://hg.mozilla.org/releases/mozilla-beta/rev/076e16981f2f
status-firefox18: --- → fixed
status-firefox19: --- → fixed
status-firefox20: --- → fixed
You need to log in before you can comment on or make changes to this bug.