Closed
Bug 422546
Opened 16 years ago
Closed 16 years ago
Topcrash [@ nsEditor::nsEditor] -- trying to addref a garbage gTypingTxnName?
Categories
(Core :: DOM: Editor, defect)
Tracking
()
VERIFIED
FIXED
mozilla1.9beta5
People
(Reporter: jruderman, Assigned: dbaron)
Details
(Keywords: crash, topcrash)
Crash Data
Attachments
(1 file)
7.58 KB,
patch
|
timeless
:
review+
roc
:
superreview+
damons
:
approval1.9+
|
Details | Diff | Splinter Review |
According to http://crash-stats.mozilla.com/topcrasher/byversion/Firefox/3.0b4 [@ nsEditor::nsEditor] is topcrash #33 overall and topcrash #10 on mac. Example: bp-d114602b-f082-11dc-9746-001a4bd43ef6 http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/editor/libeditor/base/nsEditor.cpp&rev=1.508&mark=167,180-186#167 The assumption in the highlighted comment does not hold, as you can see by adding #ifdef DEBUG nsITransactionManager *tm = mTxnMgr; PRUint32 v; NS_ADDREF(tm); NS_RELEASE2(tm, v); NS_ASSERTION(v == 1, "Expect to deref a garbage gTypingTxnName later!"); #endif and then simply opening and closing Firefox. This "optimization" of holding onto an atom for the string "Typing" seems kinda sketchy to me, but I'm not very familiar with atoms or editor code.
Flags: blocking1.9?
Assignee | ||
Comment 1•16 years ago
|
||
Yeah, I was just looking at the topcrash stats and noticed this one -- the fix is obvious. This code is broken and will crash if anybody else atomizes the same string and refcounts it such that the other user drops its last reference after the number of editors goes to zero, and then we create another editor.
Assignee: nobody → dbaron
Assignee | ||
Comment 2•16 years ago
|
||
This is around the #10 topcrash on Mac for Firefox 3.0b4 -- and the second highest that's not already fixed and not extension/plugin-related. (Mac is somewhat less cluttered with extension-related and plugin-related stuff.)
Assignee | ||
Comment 3•16 years ago
|
||
I just used nsGkAtoms since editor is part of gklayout now. (Editor has an atom list already in nsEditProperty, but it's only compiled when HTML-editing is enabled, so using that would break --enable-plaintext-editor-only.)
Attachment #309040 -
Flags: superreview?(roc)
Attachment #309040 -
Flags: review?
Attachment #309040 -
Flags: review? → review+
Comment on attachment 309040 [details] [diff] [review] patch ouch
Attachment #309040 -
Flags: superreview?(roc) → superreview+
Assignee | ||
Comment 5•16 years ago
|
||
Comment on attachment 309040 [details] [diff] [review] patch Low risk fix to a topcrash affecting Mac: probably after closing the last window and then opening more windows again, if documents containing certain tag or attribute names -- "Typing", "IME", or "Deleting" (or something else atomized) were loaded in the last window closed.
Attachment #309040 -
Flags: approval1.9?
Comment 6•16 years ago
|
||
Comment on attachment 309040 [details] [diff] [review] patch a1.9+=damons
Attachment #309040 -
Flags: approval1.9? → approval1.9+
Assignee | ||
Comment 7•16 years ago
|
||
Fix checked in to trunk, 2008-03-13 11:54 -0700.
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9beta5
Reporter | ||
Comment 8•16 years ago
|
||
This patch fixed the topcrash. No crashes were reported in 2008-03-14 through 2008-03-17 nightly builds, down from 3-4 per Mac build.
Status: RESOLVED → VERIFIED
Updated•13 years ago
|
Crash Signature: [@ nsEditor::nsEditor]
You need to log in
before you can comment on or make changes to this bug.
Description
•