Closed
Bug 141796
Opened 22 years ago
Closed 22 years ago
Mozilla crashed in http-pipelining code Trunk M1RC2 [@ nsHttpPipeline::GetRequestSize_Locked]
Categories
(Core :: Networking: HTTP, defect, P1)
Core
Networking: HTTP
Tracking
()
VERIFIED
FIXED
mozilla1.0.1
People
(Reporter: francis.uy, Assigned: darin.moz)
References
Details
(Keywords: crash, qawanted, topcrash, Whiteboard: [pipelining][adt2 RTM][driver:shaver][fixed trunk])
Crash Data
Attachments
(3 files)
2.69 KB,
text/plain
|
Details | |
2.64 KB,
text/plain
|
Details | |
2.81 KB,
patch
|
jag+mozilla
:
review+
rpotts
:
superreview+
shaver
:
approval+
|
Details | Diff | Splinter Review |
From Bugzilla Helper: User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:1.0.0+) Gecko/20020502 BuildID: 20020428 Apologies for this horribly vague report. Too bad OS X doesn't have Talkback. Beats me why it crashed. I had a few tabs open and a page loading. I wasn't clicking or scrolling the mouse. Crash log follows. Reproducible: Didn't try
Comment 2•22 years ago
|
||
-> Networking:http (nsHttpPipeline::GetRequestSize_Locked) Have you enabled Http Pipelining ?
Assignee: Matti → darin
Severity: normal → critical
Component: Browser-General → Networking: HTTP
Keywords: crash
QA Contact: imajes-qa → tever
Assignee | ||
Comment 3•22 years ago
|
||
yup, that code is only executed when pipelining is enabled.
Whiteboard: [pipelining]
Yes, I turned on pipelining a few months ago. Mozilla's Pipelining FAQ mainly talks about problems faced by the web server. It doesn't say anything about potential browser crashes. Should it?
Assignee | ||
Comment 5•22 years ago
|
||
no, clearly you've discovered a mozilla bug. mozilla should not crash even if the server is misbehaving. investigating this one for moz 1.0.1
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Priority: -- → P1
Target Milestone: --- → mozilla1.0.1
Updated•22 years ago
|
Summary: Mozilla crashed for reasons unknown → Mozilla crashed in http-pipelining code
Whiteboard: [pipelining]
This one has turned up as a topcrasher in M1RC2 with 60+ crashes. Any chance for this one before the proposed M1.0.1? nsHttpPipeline::GetRequestSize_Locked [d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpPipeline.cpp line 681] nsHttpPipeline::OnDataWritable [d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpPipeline.cpp line 432] nsHttpConnection::OnDataWritable [d:\builds\seamonkey\mozilla\netwerk\protocol\http\src\nsHttpConnection.cpp line 670] nsSocketWriteRequest::OnWrite [d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 2958] nsSocketTransport::doReadWrite [d:\builds\seamonkey\mozilla\netwerk\base\src\nsSocketTransport.cpp line 1165]
Assignee | ||
Updated•22 years ago
|
Whiteboard: [pipelining]
Assignee | ||
Updated•22 years ago
|
Target Milestone: mozilla1.0.1 → ---
Assignee | ||
Updated•22 years ago
|
Whiteboard: [pipelining] → [pipelining][adt2 RTM]
Target Milestone: --- → mozilla1.0.1
Assignee | ||
Comment 8•22 years ago
|
||
turns out that a transaction can be canceled (and therefore removed from the pipeline) before the pipeline is even written to the network. this situation leads to a crash, because the code that writes the request to the pipeline assumed all of the transactions would be valid. so the fix is very trivial... just some null pointer checks in the right places.
Comment 9•22 years ago
|
||
Comment on attachment 83655 [details] [diff] [review] v1 patch looks good. sr=rpotts@netscape.com
Attachment #83655 -
Flags: superreview+
Comment 10•22 years ago
|
||
btw... can the following assertion also be removed :-) @@ -370,7 +370,8 @@ // no need to be inside the lock for (PRInt8 i=0; i<mNumTrans; ++i) { NS_ASSERTION(mTransactionQ[i], "no transaction"); - mTransactionQ[i]->SetConnection(this); + if (mTransactionQ[i]) + mTransactionQ[i]->SetConnection(this); } } since it is now acceptable for mTransactionQ[i] to be null :-) thanks, -- rick
Assignee | ||
Updated•22 years ago
|
Whiteboard: [pipelining][adt2 RTM] → [pipelining][adt2 RTM][driver:shaver]
Assignee | ||
Comment 11•22 years ago
|
||
rick: actually in that case mTransactionQ[i] should not be null, but i'm adding a runtime check for added safety. it should not be null because there is no opportunity to cancel any of the transactions between nsHttpPipeline:: AppendTransaction and nsHttpPipeline::SetConnection.
Comment 12•22 years ago
|
||
Comment on attachment 83655 [details] [diff] [review] v1 patch r=jag
Attachment #83655 -
Flags: review+
Assignee | ||
Comment 13•22 years ago
|
||
fixed-on-trunk
Whiteboard: [pipelining][adt2 RTM][driver:shaver] → [pipelining][adt2 RTM][driver:shaver][fixed trunk]
Comment on attachment 83655 [details] [diff] [review] v1 patch a=shaver+scc+jesup for the rc3 branch. Booyah!
Attachment #83655 -
Flags: approval+
Assignee | ||
Comment 15•22 years ago
|
||
fixed-on-branch
Comment 16•22 years ago
|
||
last talkback reports of this crash were from build 2002051408 or 6 days ago verified
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
Crash Signature: [@ nsHttpPipeline::GetRequestSize_Locked]
You need to log in
before you can comment on or make changes to this bug.
Description
•