549260 - crash [@ nsTextInputListener::EditAction()][@ nsTextInputListener::EditAction ] while typing, pasting text in editor

Status: RESOLVED FIXED

(Core :: Layout: Form Controls, defect)

Description

[timeless]

Signature	nsTextInputListener::EditAction()
UUID	4ef1394b-8f61-4fea-968d-2d5c32100219
Time 	2010-02-19 13:43:17.416241
Uptime	89818
Last Crash	93113 seconds before submission
Product	Firefox
Version	3.7a2pre
Build ID	20100218045818
Branch	1.9.3
OS	Windows NT
OS Version	6.1.7600
CPU	x86
CPU Info	AuthenticAMD family 16 model 4 stepping 2
Crash Reason	EXCEPTION_ACCESS_VIOLATION
Crash Address	0x14
User Comments	
Processor Notes 	
Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsTextInputListener::EditAction 	layout/forms/nsTextControlFrame.cpp:464
1 	xul.dll 	nsEditor::NotifyEditorObservers 	editor/libeditor/base/nsEditor.cpp:1818
2 	xul.dll 	nsEditor::EndPlaceHolderTransaction 	editor/libeditor/base/nsEditor.cpp:1032
3 	xul.dll 	nsAutoPlaceHolderBatch::~nsAutoPlaceHolderBatch 	editor/libeditor/base/nsEditorUtils.h:65
4 	xul.dll 	nsPlaintextEditor::InsertText 	editor/libeditor/text/nsPlaintextEditor.cpp:801
5 	xul.dll 	nsTextControlFrame::SetValue 	layout/forms/nsTextControlFrame.cpp:2710
6 	xul.dll 	nsTextControlFrame::SetFormProperty 	layout/forms/nsTextControlFrame.cpp:1882
7 	xul.dll 	nsHTMLTextAreaElement::SetValueInternal 	content/html/content/src/nsHTMLTextAreaElement.cpp:462
8 	xul.dll 	nsHTMLTextAreaElement::SetValue 	content/html/content/src/nsHTMLTextAreaElement.cpp:480
9 	xul.dll 	nsIDOMHTMLTextAreaElement_SetValue 	obj-firefox/js/src/xpconnect/src/dom_quickstubs.cpp:16473
10 	mozjs.dll 	JSScopeProperty::set 	js/src/jsscope.h:869
11 	mozjs.dll 	js_SetPropertyHelper 	js/src/jsobj.cpp:5388
12 	mozjs.dll 	js_Interpret 	js/src/jsops.cpp:1899
13 	mozjs.dll 	js_Execute 	js/src/jsinterp.cpp:1684
14 	mozjs.dll 	JS_EvaluateUCScriptForPrincipals 	js/src/jsapi.cpp:5080
15 	xul.dll 	nsJSContext::EvaluateString 	dom/base/nsJSEnvironment.cpp:1748
16 	xul.dll 	nsScriptLoader::EvaluateScript 	content/base/src/nsScriptLoader.cpp:713
17 	xul.dll 	nsScriptLoader::ProcessRequest 	content/base/src/nsScriptLoader.cpp:626
18 	xul.dll 	nsCOMArray_base::RemoveObject 	obj-firefox/xpcom/build/nsCOMArray.cpp:129
19 	xul.dll 	nsScriptLoader::ProcessPendingRequests 	


436 nsTextInputListener::EditAction()
442   mFrame->GetEditor(getter_AddRefs(editor));

mFrame was not null here.

456     UpdateTextInputCommands(NS_LITERAL_STRING("undo"));

this calls out to arbitrary unpredictable code via domWindow->UpdateCommands(commandsToUpdate);

somehow this reaches nsTextControlFrame::PreDestroy()
my guess is nsDocShellEditorData::SetEditor() or a relative

464   mFrame->SetValueChanged(PR_TRUE);

mFrame is null here (0x14 is the right offset for functions not from some other vtable).

Comment 1

[timeless]

Attachment: proposal

Comment 2

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 429457 [details] [diff] [review]
proposal

Is the reason to not use nsWeakFrame that mFrame can change without being destroyed here?  I'd think nsWeakFrame makes it clearer what we're dealing with, if not.

Comment 3

[timeless]

Attachment: like this?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Does that patch even compile?  What's mWeakFrame?

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 430854 [details] [diff] [review]
like this?

Per comment 4

Comment 6

[Scoobidiver (away)]

It is #59 top crasher in 4.0b6 for the last 2 weeks.

Is anybody working on a recent patch ?

Comment 7

[Scoobidiver (away)]

It is #88 top crasher in 4.0b8pre for the last week. It happens also in 3.6.12.

Here are some comments:
"crashes when using JIRA 4.2"
"started typing in a field"
"pasted text into a form"

Stack traces look like:
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsTextInputListener::EditAction 	
1 	xul.dll 	nsEditor::EndPlaceHolderTransaction 	editor/libeditor/base/nsEditor.cpp:962
2 	xul.dll 	nsAutoPlaceHolderBatch::~nsAutoPlaceHolderBatch 	editor/libeditor/base/nsEditorUtils.h:65
3 	xul.dll 	nsPlaintextEditor::TypedText 	editor/libeditor/text/nsPlaintextEditor.cpp:440
4 	xul.dll 	nsPlaintextEditor::HandleKeyPressEvent 	editor/libeditor/text/nsPlaintextEditor.cpp:416

Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsTextInputListener::EditAction 	
1 	xul.dll 	nsEditor::EndPlaceHolderTransaction 	editor/libeditor/base/nsEditor.cpp:962
2 	xul.dll 	nsAutoPlaceHolderBatch::~nsAutoPlaceHolderBatch 	editor/libeditor/base/nsEditorUtils.h:65
3 	xul.dll 	nsPlaintextEditor::TypedText 	editor/libeditor/text/nsPlaintextEditor.cpp:440
4 	xul.dll 	nsPlaintextEditor::HandleKeyPressEvent 	editor/libeditor/text/nsPlaintextEditor.cpp:416

More reports at:
http://crash-stats.mozilla.com/report/list?product=Firefox&query_search=signature&query_type=exact&query=&range_value=4&range_unit=weeks&hang_type=any&process_type=any&plugin_field=&plugin_query_type=&plugin_query=&do_query=1&admin=&signature=nsTextInputListener%3A%3AEditAction%28%29

Comment 8

[Scoobidiver (away)]

It is #29 top crasher on Mac OS X in 4.0b7 for the last 2 weeks.
It is #53 top crasher on Linux in 4.0b7 for the last 2 weeks.

Comment 9

[Scoobidiver (away)]

It is #17 top crasher on Mac OS X in 4.0b8 for the last week.
It is #18 top crasher on Linux in 4.0b8 for the last week.

Comment 10

[chris hofmann]

volume looks consistent at 50-100 crashes per day over the last several months/betas, but this definitely looks like a 4.0 regression from 3.6.x

looks like this is strongly associated with hotmail and mail.live interaction.

 278 http://gfx3.hotmail.com/mail/15.4.0342.1206/ts0a.js
  35 \N
  27 about:blank
   4 http://trax.voxtelecom.co.za/secure/CreateIssue.jspa?pid=10030&issuetype=3&Create=Create
   3 https://max.max-recall.com/jira/secure/CreateIssue.jspa
   3 http://www.luobo.cc/forum.php?mod=viewthread&tid=5080744&extra=page%3D1
   3 http://bbx-nfs-jira.eac.ad.ea.com/secure/QuickSearch.jspa
   2 jar:file:///C:/Program%20Files/Mozilla%20Firefox%204.0%20Beta%207/omni.jar!/chrome/browser/content/browser/aboutHome.xhtml
   2 https://support.crbs.ucsd.edu/secure/CreateIssue.jspa
   2 https://helpdesk.clear2pay.com/helpdesk/browse/OPF-5079
   2 https://gfx7.hotmail.com/mail/15.4.0342.1206/ts0a.js
   2 https://bugs.indeed.com/secure/CreateIssue.jspa
   2 http://www.estacaopalestra.com.br/player/letreiro.htm
   2 http://sn124w.snt124.mail.live.com/mail/AttachmentUploader.aspx?_ec=1
   2 http://pdxjira.intra.infousa.com/jira/browse/OMNI-90
   2 http://mail.live.com/default.aspx?wa=wsignin1.0

and many of the form 

 wyciwyg://98/http://bl118w.blu118.mail.live.com/mail/EditMessageLight.aspx?ReadMessageId-XXXXXXXXXXXXXXXXXXXXXXXXXXXXXX


the regression starts to show up around sept. 6, 2010 which corresponds to the release of beta5.

Comment 11

[chris hofmann]

         nsTextInputListener::EditAction..
date     total    breakdown by build
         crashes  count build, count build, ...

20100901 1 3.0.192010031422 	1 , 
20100902   
20100903   
20100904   
20100905   
20100906 3  	2 4.0b52010083108, 
        		1 3.6.82010072215, 
20100907 4 4.0b52010083108 	4 , 
20100908 23 4.0b52010083108 	23 , 
20100909 21 4.0b52010083108 	21 , 
20100910 33 4.0b52010083108 	33 , 
20100911 28  	27 4.0b52010083108, 
        		1 4.0b6pre2010091004, 
20100912 21 4.0b52010083108 	21 , 
20100913 55  	53 4.0b52010083108, 
        		1 4.0b6pre2010091104, 	1 3.6.92010082415,

Comment 12

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'd block on this if we had steps to reproduce.

Comment 13

[(no longer active)]

timeless, are you working on addressing bz's comment?  If not, I can take over.

Comment 14

[(no longer active)]

OK, stealing it with timeless' permission.

Roc, do you want me to spend time on this before 2.0?

Comment 15

[Robert O'Callahan (:roc) (email my personal email if necessary)]

I'll block on it if you think you can fix it.

Comment 16

[(no longer active)]

The analysis in comment 0 is incomplete.  What happened was that this code <http://hg.mozilla.org/mozilla-central/annotate/633e895d5e84/content/html/content/src/nsTextEditorState.cpp#l843> would fire the input event, which can lead to script being executed, which, as a side effect, could invalidate the frame, which would lead to destruction of the nsTextInputListener object, which would cause a subsequent crash on <http://hg.mozilla.org/mozilla-central/annotate/633e895d5e84/content/html/content/src/nsTextEditorState.cpp#l845>.

This is what used to happen on every one of these types of crashes that I have looked at.  Bug 613979 changed the order of things such that FireOnInput would be the last thing to happen in the function, therefore inadvertently fixing this version of the crash.

However, what timeless suggested in comment 0 is still possible, and I think generally it's good hygiene to use nsWeakFrame where it's possible for us to deal with dangling frame pointers.

Patch forthcoming.

Comment 17

[(no longer active)]

Attachment: Patch (v1)

Comment 18

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 503046 [details] [diff] [review]
Patch (v1)

r=me

Comment 19

[(no longer active)]

http://hg.mozilla.org/mozilla-central/rev/8c1766aa38d3