chrome mochitest for bug 304188 leaks the world

RESOLVED FIXED in mozilla1.9

Status

()

defect
RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: ajschult784, Assigned: cpearce)

Tracking

({memory-leak, regression})

Trunk
mozilla1.9
x86
Linux
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

Posted file leak log
With current trunk SeaMonkey, the chrome mochitest for bug 304188 (chrome/toolkit/content/tests/chrome/test_bug304188.xul) leaks the world.  This started with bug 386782.  Leak log is attached.
Posted patch v1 (obsolete) — Splinter Review
* Fixed the leak. It was caused by the nsSHEntry stored in nsDocShell::mOSHE holding a raw pointer to a detached mEditorData, a non-xpcom class. The nsEditorData held xpcom-references into the editor, editor session, and the editor's listeners. Somewhere in there was a cycle or something which was preventing the nsSHEntry from being collected, causing the leak. Explicitly freeing nsDocShell's mOSHE->mEditorData and mLSHE->mEditorData in nsDocShell::Destroy() fixes the leak.
* Removes the assertions in mEditorFlags must be non-zero in nsEditingSession::DetachFromWindow()/ReattachToWindow(). mEditorFlags can be zero if we have a non-interactive HTMLEditor. This occurs in the testcase that was leaking.
Assignee: nobody → chris
Status: NEW → ASSIGNED
Attachment #318739 - Flags: review?(peterv)
Blocks: 430615
Attachment #318739 - Flags: superreview+
Attachment #318739 - Flags: review?(peterv)
Attachment #318739 - Flags: review+
Comment on attachment 318739 [details] [diff] [review]
v1

Requesting approval 1.9. This is a low risk patch which stops a memory leak.
Attachment #318739 - Flags: approval1.9?
Comment on attachment 318739 [details] [diff] [review]
v1

a1.9+=damons
Attachment #318739 - Flags: approval1.9? → approval1.9+
Keywords: checkin-needed
Whiteboard: [needs landing]
Attachment #318739 - Attachment is obsolete: true
Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.902; previous revision: 1.901
done
Checking in editor/composer/src/nsEditingSession.cpp;
/cvsroot/mozilla/editor/composer/src/nsEditingSession.cpp,v  <--  nsEditingSession.cpp
new revision: 1.63; previous revision: 1.62
done
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Whiteboard: [needs landing]
Target Milestone: --- → mozilla1.9
Backed out to see if this is causing crashes.
Status: RESOLVED → REOPENED
Keywords: checkin-needed
Resolution: FIXED → ---
Did this cause crashes?
Looks like those were due to bug 432492. We should reland this when the tree reopens.
Relanded.

Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.904; previous revision: 1.903
done
Checking in editor/composer/src/nsEditingSession.cpp;
/cvsroot/mozilla/editor/composer/src/nsEditingSession.cpp,v  <--  nsEditingSession.cpp
new revision: 1.65; previous revision: 1.64
done
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Backed out again, as we're crashing again. Either this patch or bug 422172 is causing the crashes.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Relanded (again). I forgot to repackage jemalloc, so I bet that was causing the crashes. I don't know for sure yet, though.

Checking in docshell/base/nsDocShell.cpp;
/cvsroot/mozilla/docshell/base/nsDocShell.cpp,v  <--  nsDocShell.cpp
new revision: 1.906; previous revision: 1.905
done
Checking in editor/composer/src/nsEditingSession.cpp;
/cvsroot/mozilla/editor/composer/src/nsEditingSession.cpp,v  <--  nsEditingSession.cpp
new revision: 1.67; previous revision: 1.66
done
Status: REOPENED → RESOLVED
Closed: 11 years ago11 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.