SchemeIs failure leaks nsHttpChannel

RESOLVED FIXED in mozilla1.8alpha2

Status

()

RESOLVED FIXED
15 years ago
14 years ago

People

(Reporter: Biesinger, Assigned: darin.moz)

Tracking

({fixed-aviary1.0, fixed1.7.5, memory-leak})

Trunk
mozilla1.8alpha2
x86
Linux
fixed-aviary1.0, fixed1.7.5, memory-leak
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(1 attachment)

in nsHttpHandler::NewProxiedChannel:
1346     NS_NEWXPCOM(httpChannel, nsHttpChannel);
1347     if (!httpChannel)
1348         return NS_ERROR_OUT_OF_MEMORY;
1349     NS_ADDREF(httpChannel);
1350 
1351     nsresult rv;
1352 
1353     PRBool https;
1354     rv = uri->SchemeIs("https", &https);
1355     if (NS_FAILED(rv)) return rv;

so if SchemeIs fails, httpChannel will never get deleted.
I'd suggest moving the schemeIs call above NS_NEWXPCOM.
(Assignee)

Updated

15 years ago
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla1.8alpha2
(Assignee)

Comment 1

15 years ago
Created attachment 150524 [details] [diff] [review]
v1 patch
(Assignee)

Updated

15 years ago
Attachment #150524 - Flags: review?(cbiesinger)
Attachment #150524 - Flags: review?(cbiesinger) → review+
(Assignee)

Comment 2

15 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 15 years ago
Resolution: --- → FIXED

Comment 3

15 years ago
It is a fixed memory leak bug, could it be considered for 1.7.1 ?
Flags: blocking1.7.1?
(Assignee)

Updated

15 years ago
Attachment #150524 - Flags: approval1.7.1?
Comment on attachment 150524 [details] [diff] [review]
v1 patch

sr/a=sspitzer for 1.7.1.

curious, did someone actually see this leak, or did someone find it while
looking at the code?

my guess is that if they saw it in action, we passed in null for the first arg,
which sounds like broken code.
Attachment #150524 - Flags: superreview+
Attachment #150524 - Flags: approval1.7.1?
Attachment #150524 - Flags: approval1.7.1+
(Assignee)

Comment 5

15 years ago
fixed1.7.1
Keywords: fixed1.7.1

Updated

15 years ago
Flags: blocking1.7.1?
Keywords: mlk
someone found this while looking at the code.

> my guess is that if they saw it in action, we passed in null for the first arg,

actually no, since _that_ would've lead to a crash. well... I guess you could
call this a shutdown leak of this channel. ;)

Comment 7

15 years ago
It does not chekin to AVIARY_1_0_20040515_BRANCH.
(Assignee)

Comment 8

15 years ago
fixed-aviary1.0
Whiteboard: fixed-aviary1.0
Keywords: fixed-aviary1.0
Whiteboard: fixed-aviary1.0
You need to log in before you can comment on or make changes to this bug.