leak loading site from HTTPS

RESOLVED FIXED in mozilla1.8beta2

Status

()

Core
Networking
P1
major
RESOLVED FIXED
13 years ago
13 years ago

People

(Reporter: dbaron, Assigned: Darin Fisher)

Tracking

({mlk})

Trunk
mozilla1.8beta2
x86
Linux
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

(URL)

Attachments

(2 attachments, 1 obsolete attachment)

(Reporter)

Description

13 years ago
I leak a good bit of stuff when loading something over https, e.g.,
https://bugzilla.mozilla.org/show_bug.cgi?id=285943 .

The problem seems to be a cycle:
 * An nsNSSSocketInfo object's mCallbacks member owns (via XPCOM proxy code) an
nsHttpConnection object
 * The nsHttpConnection object's mSocketTransport, mSocketIn, and mSocketOut
members own an nsSocketTransport (and its internal objects which aggregate its
refcount)
 * The nsSocketTransport object's mSecInfo member owns the nsNSSSocketInfo.
(Reporter)

Updated

13 years ago
Keywords: mlk
(Reporter)

Comment 1

13 years ago
Created attachment 177301 [details]
edited leaksoup output showing cycle
(Reporter)

Updated

13 years ago
Severity: normal → major
(Assignee)

Comment 2

13 years ago
I think we should be null'ing out nsSocketTransport::mSecInfo at some point. 
Or, maybe since consumers may want that object after the socket has closed, it
would be better to call SetNotificationCallbacks(null) once we are closing the
connection (in OnSocketDetached most likely).
(Assignee)

Updated

13 years ago
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
(Assignee)

Comment 3

13 years ago
Created attachment 179775 [details] [diff] [review]
v1 patch
Attachment #179775 - Flags: review?(cbiesinger)
(Assignee)

Comment 4

13 years ago
biesi points out on IRC that this patch makes it so that consumers would no
longer be able to access security info for a socket after the socket has closed.
 that is probably a legitimate concern given that a consumer might not have a
chance to call GetSecurityInfo before the socket is closed in the case of a
protocol where the client writes nothing to the socket and only reads a small
chunk of data from the server before the server closes the socket.

i think we may need to look into another way to break this cycle.
(Assignee)

Updated

13 years ago
Attachment #179775 - Attachment is obsolete: true
Attachment #179775 - Flags: review?(cbiesinger)
Comment on attachment 179775 [details] [diff] [review]
v1 patch

please document in nsISocketTransport that the securityInfo can't be accessed
after the socket has closed
Attachment #179775 - Attachment is obsolete: false
Attachment #179775 - Flags: review+
(Assignee)

Comment 6

13 years ago
Created attachment 179791 [details] [diff] [review]
v2 patch
Attachment #179775 - Attachment is obsolete: true
Attachment #179791 - Flags: review?(cbiesinger)
Comment on attachment 179791 [details] [diff] [review]
v2 patch

ok, I like this much better, thanks.
Attachment #179791 - Flags: review?(cbiesinger) → review+
(Assignee)

Updated

13 years ago
Attachment #179791 - Flags: superreview?(dbaron)
(Reporter)

Comment 8

13 years ago
Comment on attachment 179791 [details] [diff] [review]
v2 patch

sr=dbaron.

(Another possibility is perhaps using weak references somewhere...)
Attachment #179791 - Flags: superreview?(dbaron) → superreview+
(Assignee)

Comment 9

13 years ago
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Last Resolved: 13 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.