Last Comment Bug 375196 - crashes [@ nsVoidArray::RemoveElementsAt][@ nsTextControlFrame::FireOnInput]
: crashes [@ nsVoidArray::RemoveElementsAt][@ nsTextControlFrame::FireOnInput]
[sg:critical?] null-deref on 1.8 branch?
: crash, fixed1.8.0.12, fixed1.8.1.4, testcase
Product: Core
Classification: Components
Component: Event Handling (show other bugs)
: Trunk
: x86 All
-- critical (vote)
: ---
Assigned To: Olli Pettay [:smaug]
: Hixie (not reading bugmail)
: Andrew Overholt [:overholt]
Depends on: 375093
Blocks: 374083 376110
  Show dependency treegraph
Reported: 2007-03-24 08:45 PDT by Olli Pettay [:smaug]
Modified: 2007-08-17 10:22 PDT (History)
6 users (show)
dveditz: blocking1.8.1.4+
dveditz: wanted1.8.1.x+
dveditz: blocking1.8.0.12+
dveditz: wanted1.8.0.x+
dveditz: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

the patch (1.15 KB, patch)
2007-03-25 06:20 PDT, Olli Pettay [:smaug]
no flags Details | Diff | Splinter Review
using nsCOMPtr<nsIPresShell> (21.97 KB, patch)
2007-03-25 08:12 PDT, Olli Pettay [:smaug]
roc: review+
roc: superreview+
Details | Diff | Splinter Review
for branches (24.23 KB, patch)
2007-03-27 03:05 PDT, Olli Pettay [:smaug]
roc: review+
roc: superreview+
dveditz: approval1.8.1.4+
dveditz: approval1.8.0.12+
Details | Diff | Splinter Review

Description User image Olli Pettay [:smaug] 2007-03-24 08:45:35 PDT
See bug 375093

talkback ID: TB30519322Z
MSVCR80.dll + 0x1520a (0x7814520a)
nsVoidArray::RemoveElementsAt  [mozilla/xpcom/build/nsvoidarray.cpp, line 591]
nsTextControlFrame::FireOnInput  [mozilla/layout/forms/nstextcontrolframe.cpp,
line 2472]
nsTextInputListener::EditAction  [mozilla/layout/forms/nstextcontrolframe.cpp,
line 517]
nsEditor::NotifyEditorObservers  [mozilla/editor/libeditor/base/nseditor.cpp,
line 1795]
[mozilla/editor/libeditor/base/nseditorutils.h, line 66]

Possible fix
Comment 1 User image Martijn Wargers [:mwargers] 2007-03-24 09:23:27 PDT
I can reproduce this crash with a 2007-03-24 trunk build.
But I haven't been able to reproduce it with my debug build.
If someone can reproduce this crash in its own built build and then try out Olli's patch, that would be great.
Comment 2 User image Martijn Wargers [:mwargers] 2007-03-24 09:25:03 PDT
Btw, I sometimes can reproduce this crash (in 20% of the cases) when right-clicking pasting in the text input.
Comment 3 User image Mats Palmgren (:mats) 2007-03-25 06:18:55 PDT
(In reply to comment #1)
> If someone can reproduce this crash in its own built build and then try out
> Olli's patch, that would be great.

I can reproduce this crash in my debug build and the suggested patch
fixed it.
Comment 4 User image Olli Pettay [:smaug] 2007-03-25 06:20:56 PDT
Created attachment 259578 [details] [diff] [review]
the patch
Comment 5 User image Mats Palmgren (:mats) 2007-03-25 07:21:18 PDT
Should we fix CheckFireOnChange and NotifySelectionChanged too?,2492#278

and nsListControlFrame::FireOnChange?

and perhaps document in nsIPresShell.h that callers of
HandleEventWithTarget and HandleDOMEventWithTarget
guaranties that a strong ref exist for the duration of the call.

Ditto for HandleEvent in nsIViewObserver.h (which PresShell implements)

-or- should we leave the callers as is and make the above HandleEvent
methods hold a strong ref on 'this'?
Comment 6 User image Olli Pettay [:smaug] 2007-03-25 07:30:47 PDT
Caller of HandleEventWithTarget or HandleDOMEventWithTarget should keep
strong ref. Just going through all those cases ...
Comment 7 User image Olli Pettay [:smaug] 2007-03-25 08:12:25 PDT
Created attachment 259585 [details] [diff] [review]
using nsCOMPtr<nsIPresShell>

I think we want to have a version for branches too.
Comment 8 User image Robert O'Callahan (:roc) (email my personal email if necessary) 2007-03-25 15:51:46 PDT
Comment on attachment 259585 [details] [diff] [review]
using nsCOMPtr<nsIPresShell>

     // shell may no longer be alive, don't use it here unless you keep a ref

You could remove this comment.
Comment 9 User image Olli Pettay [:smaug] 2007-03-27 03:05:55 PDT
Created attachment 259780 [details] [diff] [review]
for branches

Similar for branches.
1.8.0/nsGfxScrollFrame.cpp needs to be fixed manually when applying the
Comment 10 User image Daniel Veditz [:dveditz] 2007-03-28 10:38:10 PDT
On an unfixed trunk debug build I get a deleted PresShell, on the 1.8 branch it
appears to be a relatively safe null-deref DoS. DeCOMtamination went too far on
the trunk? Better to just fix it though, just in case.
Comment 11 User image Daniel Veditz [:dveditz] 2007-03-29 10:19:19 PDT
Comment on attachment 259780 [details] [diff] [review]
for branches

approved for and, a=dveditz for release-drivers
Comment 12 User image Martijn Wargers [:mwargers] 2007-04-03 17:25:25 PDT
Verified fixed, using:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9a4pre) Gecko/20070403 Minefield/3.0a4pre

Note You need to log in before you can comment on or make changes to this bug.