OOM crash [@ nsSocketTransport::Init]

RESOLVED FIXED

Status

()

--
major
RESOLVED FIXED
14 years ago
8 years ago

People

(Reporter: timeless, Assigned: timeless)

Tracking

({crash, helpwanted})

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

Firefox Tracking Flags

(Not tracked)

Details

(crash signature, URL)

Attachments

(4 obsolete attachments)

(Assignee)

Description

14 years ago
this is an audit, the Init methods need to complain if !mLock, and you can't 
assume strdup will succeed (strcmp will crash when you're wrong).

Comment 1

14 years ago
Any crashes show up in talkback?  Patches welcome.
Severity: critical → major
Component: Networking: HTTP → Networking
Keywords: helpwanted
(Assignee)

Comment 2

14 years ago
Created attachment 185535 [details] [diff] [review]
handle failures...
Assignee: darin → timeless
Status: NEW → ASSIGNED
(Assignee)

Comment 3

14 years ago
Created attachment 185536 [details] [diff] [review]
-w for review
Attachment #185536 - Flags: superreview?(darin)
Attachment #185536 - Flags: review?(darin)

Comment 4

14 years ago
Comment on attachment 185536 [details] [diff] [review]
-w for review

Hmm.. have you tested this with a HTTP proxy configured?  hint: there should be
no registered socket provider for a socket type of "http", which is why we only
enter the loop to process typeCount socket types.

Comment 5

14 years ago
Comment on attachment 185536 [details] [diff] [review]
-w for review

OK, my previous comment can be ignored.  I forgot that we
treat proxy types of "http" and "direct" as NULL before
entering this block of code that you are changing.


>Index: mozilla/netwerk/base/src/nsSocketTransport2.cpp

>+        if (!mTypes[i]) {
>+            mTypeCount = i - 1;
>+            return NS_ERROR_OUT_OF_MEMORY;
>+        }

What if |i == 0| when we hit this condition?

Comment 6

14 years ago
Comment on attachment 185536 [details] [diff] [review]
-w for review

I recall that timeless proposed a reasonable fix for this problem over IRC. 
Marking review- to get attention.
Attachment #185536 - Flags: superreview?(darin)
Attachment #185536 - Flags: review?(darin)
Attachment #185536 - Flags: review-
(Assignee)

Comment 7

14 years ago
Created attachment 186466 [details] [diff] [review]
-w for review. special case i = 0
Attachment #185535 - Attachment is obsolete: true
Attachment #185536 - Attachment is obsolete: true
Attachment #186466 - Flags: superreview?(darin)
Attachment #186466 - Flags: review?(darin)

Comment 8

14 years ago
Comment on attachment 186466 [details] [diff] [review]
-w for review. special case i = 0

>Index: mozilla/netwerk/base/src/nsSocketTransport2.cpp

>+        if (!mTypes[i]) {
>+            mTypeCount = i ? i - 1 : 0;
>+            return NS_ERROR_OUT_OF_MEMORY;

Actually, shouldn't this be:  "mTypeCount = i;"

Afterall, if the element at index i is null, then it means that we have
i valid elements.  "The length of a string indexes the null terminator."
Attachment #186466 - Flags: superreview?(darin)
Attachment #186466 - Flags: review?(darin)
Attachment #186466 - Flags: review-
(Assignee)

Comment 9

14 years ago
Created attachment 186539 [details] [diff] [review]
-w for review. no special casing!

oops, indeed you're right, thanks :)
(Assignee)

Updated

14 years ago
Attachment #186466 - Attachment is obsolete: true
Attachment #186539 - Flags: superreview?(darin)
Attachment #186539 - Flags: review?(darin)

Updated

14 years ago
Attachment #186539 - Flags: superreview?(darin)
Attachment #186539 - Flags: superreview+
Attachment #186539 - Flags: review?(darin)
Attachment #186539 - Flags: review+
(Assignee)

Updated

14 years ago
Attachment #186539 - Flags: approval1.8b3?

Updated

14 years ago
Attachment #186539 - Flags: approval1.8b3? → approval1.8b3+
(Assignee)

Comment 10

14 years ago
Comment on attachment 186539 [details] [diff] [review]
-w for review. no special casing!

mozilla/netwerk/base/src/nsSocketTransport2.cpp 	1.34
Attachment #186539 - Attachment is obsolete: true
(Assignee)

Updated

14 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
Crash Signature: [@ nsSocketTransport::Init]
You need to log in before you can comment on or make changes to this bug.