Bugzilla@Mozilla – Bug 614499
Crash [@ nsSHTransaction::GetPrev ]
Last modified: 2011-06-20 18:08:56 PDT
Summon comment box
Created attachment 492931 [details] testcase (crashes Firefox when loaded) This might be the same bug as topcrash bug 612881 / bug 612887.
Created attachment 492932 [details] 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.
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.
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.
Created attachment 492984 [details] [review] patch, clear the pointers when modifying mListRoot
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] [review] patch, clear the pointers when modifying mListRoot r=me
The patch applies cleanly to branches. I'm not sure whether we really need it there, but should be safe.
http://hg.mozilla.org/mozilla-central/rev/92facbe5957e
Comment on attachment 492984 [details] [review] patch, clear the pointers when modifying mListRoot Approved for 1.9.2.14 and 1.9.1.17, a=dveditz for release-drivers
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/36da01644e41 http://hg.mozilla.org/releases/mozilla-1.9.2/rev/12d4747b0a6a
Backed out from branches, since there we some strange errors. Not sure whether those were caused by the patch or by something else.
Seems like a valid problem. Investigating.
Ok, need to take one null check from Bug 534178 to branches.
Created attachment 500990 [details] [review] null check aNext
Comment on attachment 500990 [details] [review] null check aNext r=me
Comment on attachment 500990 [details] [review] null check aNext Approving updated patch
(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.
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5873543f363c http://hg.mozilla.org/releases/mozilla-1.9.2/rev/34157257960e
Are there any STR for branches? I see that it is noted that the testcase doesn't work there.
Crashtest: http://hg.mozilla.org/mozilla-central/rev/bed7a9570b40