Closed
Bug 292972
Opened 21 years ago
Closed 21 years ago
Leak WindowStateHolder if CopyJSProperties() fails
Categories
(Core :: DOM: Core & HTML, defect)
Core
DOM: Core & HTML
Tracking
()
RESOLVED
FIXED
People
(Reporter: bzbarsky, Assigned: mikael)
References
Details
(Keywords: memory-leak)
Attachments
(1 file)
|
983 bytes,
patch
|
bzbarsky
:
review+
bryner
:
superreview+
brendan
:
approval1.8b2+
|
Details | Diff | Splinter Review |
The code looks like:
+ *aState = new WindowStateHolder(cx, stateObj, this);
+ NS_ENSURE_TRUE(*aState, NS_ERROR_OUT_OF_MEMORY);
+ nsresult rv = CopyJSProperties(cx, mJSObject, stateObj);
+ NS_ENSURE_SUCCESS(rv, rv);
We need to delete *aState (and null it out, probably) if rv is a failure there.
| Reporter | ||
Updated•21 years ago
|
Blocks: blazinglyfastback
| Assignee | ||
Comment 1•21 years ago
|
||
Assignee: general → mikael
Status: NEW → ASSIGNED
Attachment #182868 -
Flags: superreview?(bryner)
Attachment #182868 -
Flags: review?(bzbarsky)
Comment 2•21 years ago
|
||
Comment on attachment 182868 [details] [diff] [review]
fix
>Index: dom/src/base/nsGlobalWindow.cpp
>===================================================================
>RCS file: /cvsroot/mozilla/dom/src/base/nsGlobalWindow.cpp,v
>retrieving revision 1.735
>diff -u -p -8 -r1.735 nsGlobalWindow.cpp
>--- dom/src/base/nsGlobalWindow.cpp 4 May 2005 23:42:35 -0000 1.735
>+++ dom/src/base/nsGlobalWindow.cpp 7 May 2005 10:55:01 -0000
>@@ -6010,17 +6010,21 @@ nsGlobalWindow::SaveWindowState(nsISuppo
> // The window state object will root the JSObject.
> *aState = new WindowStateHolder(cx, stateObj, this);
> NS_ENSURE_TRUE(*aState, NS_ERROR_OUT_OF_MEMORY);
>
> #ifdef DEBUG_PAGE_CACHE
> printf("saving window state, stateObj = %p\n", stateObj);
> #endif
> nsresult rv = CopyJSProperties(cx, mJSObject, stateObj);
>- NS_ENSURE_SUCCESS(rv, rv);
>+ if(NS_FAILED(rv)) {
Space after |if| to keep with the style.
Looks ok otherwise, but I'm curious if you've found a situation where
CopyJSProperties fails?
Attachment #182868 -
Flags: superreview?(bryner) → superreview+
| Reporter | ||
Updated•21 years ago
|
Attachment #182868 -
Flags: review?(bzbarsky) → review+
| Reporter | ||
Comment 3•21 years ago
|
||
Comment on attachment 182868 [details] [diff] [review]
fix
Requesting 1.8b2 approval for this simple failure-case leak fix...
Attachment #182868 -
Flags: approval1.8b2?
Updated•21 years ago
|
Attachment #182868 -
Flags: approval1.8b2? → approval1.8b2+
| Reporter | ||
Comment 4•21 years ago
|
||
Mikael, let me know if you need this checked in, ok?
Comment 6•21 years ago
|
||
checked in.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
| Reporter | ||
Updated•21 years ago
|
Flags: blocking1.8b3?
Updated•7 years ago
|
Component: DOM → DOM: Core & HTML
You need to log in
before you can comment on or make changes to this bug.
Description
•