OOM crash [@nsStyleContext::ApplyStyleFixups]

RESOLVED FIXED

Status

()

--
critical
RESOLVED FIXED
14 years ago
13 years ago

People

(Reporter: dewildt, Assigned: bastiaan)

Tracking

({crash, helpwanted})

Trunk
x86
Windows XP
crash, helpwanted
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(crash signature)

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

14 years ago
GetUniqueStyleData could return nsnull is several cases (e.g. invalid
nsStyleStructID, OOM etc.). This is not handled in
nsStyleContext::ApplyStyleFixups which could result in a crash
Hmmmm...  We really need this fixup -- if it doesn't happen, we'll crash in
various places in layout, iirc.  Maybe if getting the unique style data fails we
should just munge our non-unique data?
Keywords: helpwanted
(Assignee)

Comment 2

13 years ago
Created attachment 187730 [details] [diff] [review]
add null check

This patch adds a null check. I don't completely understand bz's comment, so
his suggested additional fix is lost upon me.
(Assignee)

Updated

13 years ago
Attachment #187730 - Flags: review?(dbaron)
My comment just says that with that patch we'll go on to crash elsewhere instead
of crashing here.

My suggestion is that if we fail to get a unique style data we should perhaps
modify |disp| (or rather a non-const version thereof) directly.  Possibly by
making GetUniqueStyleData never return null and return the non-unique data if it
can't allocate a new struct.
(Assignee)

Updated

13 years ago
Attachment #187730 - Flags: review?(dbaron)
(Assignee)

Comment 4

13 years ago
Created attachment 188117 [details] [diff] [review]
better fix
Attachment #187730 - Attachment is obsolete: true
(Assignee)

Comment 5

13 years ago
Created attachment 188120 [details] [diff] [review]
even better fix..

On second thought, we should probably return immediately.
(Assignee)

Updated

13 years ago
Assignee: dbaron → b.jacques
Attachment #188117 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #188120 - Flags: superreview?(dbaron)
Attachment #188120 - Flags: review?(dbaron)
Comment on attachment 188120 [details] [diff] [review]
even better fix..

>+    NS_WARNING("Ran out of memory while trying to allocate memory for a unique nsStyleStruct! " \
>+               "Returning the non-unique data.");

No need for the backslash.  It may even be counterproductive.
Attachment #188120 - Flags: superreview?(dbaron)
Attachment #188120 - Flags: superreview+
Attachment #188120 - Flags: review?(dbaron)
Attachment #188120 - Flags: review+
(Assignee)

Comment 7

13 years ago
Created attachment 189720 [details] [diff] [review]
patch for checkin
Attachment #188120 - Attachment is obsolete: true
Attachment #189720 - Flags: approval1.8b4?
Comment on attachment 188120 [details] [diff] [review]
even better fix..

Requesting 1.8b4 approval for this OOM error handling fix.  Risk is quite low.
Attachment #188120 - Attachment is obsolete: false
Attachment #188120 - Flags: approval1.8b4?
Attachment #188120 - Flags: approval1.8b4?

Updated

13 years ago
Attachment #189720 - Flags: approval1.8b4? → approval1.8b4+
(Assignee)

Comment 9

13 years ago
Checked in by timeless (2005-07-19 14:03).
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
Crash Signature: [@nsStyleContext::ApplyStyleFixups]
You need to log in before you can comment on or make changes to this bug.