Closed Bug 614499 Opened 14 years ago Closed 14 years ago

Crash [@ nsSHTransaction::GetPrev ]

Categories

(Core :: DOM: Navigation, defect)

Product:
Core
Component:
DOM: Navigation
Platform:
x86
Windows 7
Type:
defect
Priority:
Not set
Severity:
critical

Tracking

()

Status:
RESOLVED FIXED
Tracking Flags:
Tracking Status
blocking2.0 --- final+
blocking1.9.2 --- .14+
status1.9.2 --- .14-fixed
blocking1.9.1 --- .17+
status1.9.1 --- .17-fixed

People

(Reporter: jruderman, Assigned: smaug)

References

Details

(Keywords: crash, testcase, Whiteboard: [sg:critical] testcase doesn't affect < FF4 [qa-needs-STR])

Crash Data

Attachments

(3 files, 1 obsolete file)

testcase (crashes Firefox when loaded)
341 bytes, text/html
Details
stack trace
12.57 KB, text/plain
Details
null check aNext
1.62 KB, patch
bzbarsky
: review+
christian
: approval1.9.2.14+
christian
: approval1.9.1.17+
Details | Diff | Splinter Review
This might be the same bug as topcrash bug 612881 / bug 612887.
Attached file stack trace —
The top frame of the stack is random, so this could be anywhere between topcrash #1 and topcrash #11. Fixing it in beta8 would be nice.
blocking2.0: --- → ?
Is this win7 only? It doesn't crash my Linux x64 box. Will definitely look at this tomorrow.
Debug builds crash on Windows, Mac, and Linux. Opt builds crash on at least Windows and Mac.
Assignee: nobody → Olli.Pettay
Justin, I took this before noticing your comment. I assume this might be a regression from my changes.
Ah, the code causing this has been there for ages, at least from year 2001.
Paul, does session restore get nsISHTransaction.prev anywhere in the branches? C++ does seem to call ::GetPrev in 1.9.1 nor 1.9.2.
(In reply to comment #8) > Paul, does session restore get nsISHTransaction.prev anywhere in the branches? > C++ does seem to call ::GetPrev in 1.9.1 nor 1.9.2. I didn't find any calls from sessionstore. I don't think it's ever used nsISHTransaction.
Ok, thanks. I think in that case it isn't absolutely critical to take this to branches. But for trunk we really need this.
Comment on attachment 492984 [details] [diff] [review] patch, clear the pointers when modifying mListRoot r=me
Attachment #492984 - Flags: review?(bzbarsky) → review+
Attachment #492984 - Flags: approval2.0?
Attachment #492984 - Flags: approval2.0?
Summary: Crash [@ nsSHTransaction::GetPrev] → Crash [@ nsSHTransaction::GetPrev ]
The patch applies cleanly to branches. I'm not sure whether we really need it there, but should be safe.
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
http://hg.mozilla.org/mozilla-central/rev/92facbe5957e
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
blocking1.9.1: ? → ---
blocking1.9.2: ? → ---
status1.9.1: --- → wanted
status1.9.2: --- → wanted
Comment on attachment 492984 [details] [diff] [review] patch, clear the pointers when modifying mListRoot Approved for 1.9.2.14 and 1.9.1.17, a=dveditz for release-drivers
Attachment #492984 - Flags: approval1.9.2.14+
Attachment #492984 - Flags: approval1.9.1.17+
Whiteboard: [sg:critical] → [sg:critical] testcase doesn't affect < FF4
blocking1.9.1: --- → needed
blocking1.9.2: --- → needed
blocking1.9.1: needed → .17+
blocking1.9.2: needed → .14+
Backed out from branches, since there we some strange errors. Not sure whether those were caused by the patch or by something else.
status1.9.1: .17-fixed → ?
status1.9.2: .14-fixed → ?
Seems like a valid problem. Investigating.
Ok, need to take one null check from Bug 534178 to branches.
Attached patch null check aNext — — Splinter Review
Attachment #500990 - Flags: review?(bzbarsky)
Attachment #500990 - Flags: approval1.9.2.14?
Attachment #500990 - Flags: approval1.9.1.17?
No longer blocks: 612881
Blocks: 612881
Comment on attachment 500990 [details] [diff] [review] null check aNext r=me
Attachment #500990 - Flags: review?(bzbarsky) → review+
Comment on attachment 500990 [details] [diff] [review] null check aNext Approving updated patch
Attachment #500990 - Flags: approval1.9.2.14?
Attachment #500990 - Flags: approval1.9.2.14+
Attachment #500990 - Flags: approval1.9.1.17?
Attachment #500990 - Flags: approval1.9.1.17+
Attachment #492984 - Flags: approval1.9.2.14-
Attachment #492984 - Flags: approval1.9.2.14+
Attachment #492984 - Flags: approval1.9.1.17-
Attachment #492984 - Flags: approval1.9.1.17+
Attachment #492984 - Attachment is obsolete: true
(In reply to comment #16) > Backed out from branches, since there we some strange errors. > Not sure whether those were caused by the patch or by something else. This caused a huge crash spike, not something else http://crash-stats.mozilla.com/products/Firefox/versions/3.6.14pre http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=nsSHTransaction%3A%3ASetNext%28nsISHTransaction*%29&version=Firefox%3A3.6.14pre We definitely need the second patch with the null check.
Yeah. I'll push the null check patch this weekend.
Are there any STR for branches? I see that it is noted that the testcase doesn't work there.
Whiteboard: [sg:critical] testcase doesn't affect < FF4 → [sg:critical] testcase doesn't affect < FF4 [qa-needs-STR]
Group: core-security
Crash Signature: [@ nsSHTransaction::GetPrev ]
Crashtest: http://hg.mozilla.org/mozilla-central/rev/bed7a9570b40
Flags: in-testsuite+
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: