Closed
Bug 285991
Opened 19 years ago
Closed 19 years ago
leak loading site from HTTPS
Categories
(Core :: Networking, defect, P1)
Tracking
()
RESOLVED
FIXED
mozilla1.8beta2
People
(Reporter: dbaron, Assigned: darin.moz)
References
()
Details
(Keywords: memory-leak)
Attachments
(2 files, 1 obsolete file)
16.05 KB,
text/html; charset=UTF-8
|
Details | |
2.61 KB,
patch
|
Biesinger
:
review+
dbaron
:
superreview+
|
Details | Diff | Splinter Review |
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 | ||
Comment 1•19 years ago
|
||
Reporter | ||
Updated•19 years ago
|
Severity: normal → major
Assignee | ||
Comment 2•19 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•19 years ago
|
Status: NEW → ASSIGNED
Priority: -- → P1
Target Milestone: --- → mozilla1.8beta2
Assignee | ||
Comment 3•19 years ago
|
||
Attachment #179775 -
Flags: review?(cbiesinger)
Assignee | ||
Comment 4•19 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•19 years ago
|
Attachment #179775 -
Attachment is obsolete: true
Attachment #179775 -
Flags: review?(cbiesinger)
Comment 5•19 years ago
|
||
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•19 years ago
|
||
Attachment #179775 -
Attachment is obsolete: true
Attachment #179791 -
Flags: review?(cbiesinger)
Comment 7•19 years ago
|
||
Comment on attachment 179791 [details] [diff] [review] v2 patch ok, I like this much better, thanks.
Attachment #179791 -
Flags: review?(cbiesinger) → review+
Assignee | ||
Updated•19 years ago
|
Attachment #179791 -
Flags: superreview?(dbaron)
Reporter | ||
Comment 8•19 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•19 years ago
|
||
fixed-on-trunk
Status: ASSIGNED → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•