[FIX]Non-displayed invalid frames processed - bypasses recursion check resulting in memory leak / crash

RESOLVED FIXED in mozilla1.8alpha2

Status

()

P1
critical
RESOLVED FIXED
15 years ago
3 months ago

People

(Reporter: megabyte, Assigned: bzbarsky)

Tracking

(4 keywords)

Trunk
mozilla1.8alpha2
crash, fixed1.7, hang, memory-leak
Points:
---
Bug Flags:
blocking1.7 +

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: fixed-aviary1.0, URL)

Attachments

(2 attachments)

(Reporter)

Description

15 years ago
If a <frame> occurs after the number of frames specified in its containing
<frameset>, it is still processed even though it is not displayed and ought to
be invalid markup.  The real problem with this is that the recursive frame
checks (bug 8065) do not take place so a recursive frame will cause 100% CPU
usage and unbounded memory increase (until crash).
Unfortunately the Blackboard software at Virginia Tech currently does this
(you'll need an account to see the problem, so I'll attach a minimized testcase).
(Reporter)

Comment 1

15 years ago
Created attachment 149236 [details]
Reduced Testcase
(Reporter)

Comment 2

15 years ago
By the way, save and load the testcase locally for full effect since the network
can slow down the problem.  Since the testcase is reduced, there is no content
on the page; therefore, the memory leak isn't as bad as it would be on a real
world site.
(Reporter)

Updated

15 years ago
Summary: Non-displayed invalid frames processed - bypasses recursion check → Non-displayed invalid frames processed - bypasses recursion check resulting in memory leak / crash
(Reporter)

Updated

15 years ago
Keywords: hang
(Assignee)

Comment 3

15 years ago
Created attachment 149480 [details] [diff] [review]
Fix the docshell parenting

Processing the frame is right, I think.  But the way we were parenting
docshells was absolutely and totally bogus.
Assignee: nobody → bzbarsky
Status: NEW → ASSIGNED
(Assignee)

Updated

15 years ago
Attachment #149480 - Flags: superreview?(jst)
Attachment #149480 - Flags: review?(jst)
(Assignee)

Comment 4

15 years ago
Thanks for the excellent testcase, Aaron!
OS: Windows Server 2003 → All
Priority: -- → P1
Hardware: PC → All
Summary: Non-displayed invalid frames processed - bypasses recursion check resulting in memory leak / crash → [FIX]Non-displayed invalid frames processed - bypasses recursion check resulting in memory leak / crash
Target Milestone: --- → mozilla1.8alpha2
Comment on attachment 149480 [details] [diff] [review]
Fix the docshell parenting

Hmm, I wonder what kind of reason I had for that code when I originally wrote
the frame loader :-)

r+sr=jst
Attachment #149480 - Flags: superreview?(jst)
Attachment #149480 - Flags: superreview+
Attachment #149480 - Flags: review?(jst)
Attachment #149480 - Flags: review+
(Assignee)

Comment 6

15 years ago
Checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED
(Reporter)

Comment 7

15 years ago
Can this get into the branches?  I'd really hate to see this in Firefox 1.0.
Flags: blocking1.7?
(Assignee)

Comment 8

15 years ago
Comment on attachment 149480 [details] [diff] [review]
Fix the docshell parenting

Yeah, it may make sense to get this on the 1.7 branch.	It's a safe change.

For aviary, it's not clear to me what the review or checkin procedures are, so
ping the firefox folks about it if you want this in Firefox 1.0.
Attachment #149480 - Flags: approval1.7?

Comment 9

15 years ago
Comment on attachment 149480 [details] [diff] [review]
Fix the docshell parenting

a=chofmann for 1.7
Attachment #149480 - Flags: approval1.7? → approval1.7+

Updated

15 years ago
Flags: blocking1.7? → blocking1.7+

Comment 10

15 years ago
fixed on branch:
2004-06-01 08:34 bzbarsky%mit.edu mozilla/content/base/src/nsFrameLoader.cpp
1.37.2.1
Keywords: fixed1.7

Comment 11

15 years ago
for the record, mconnor checked this into the aviary branch (along with a bunch
of other 1.7 branch fixes)

Updated

15 years ago
Whiteboard: fixed-aviary1.0

Updated

3 months ago
Product: Core → Core Graveyard
Component: Layout: HTML Frames → Layout: Images
Product: Core Graveyard → Core
You need to log in before you can comment on or make changes to this bug.