880237 - crash in mozInlineSpellChecker::IgnoreWords

Status: VERIFIED FIXED

(Core :: Spelling checker, defect)

Description

[Ludovic Hirlimann [:Usul]]

This bug was filed from the Socorro interface and is 
report bp-7fb33276-e0a3-4da1-8392-21ae42130606 .
============================================================= 
0 	XUL 	mozInlineSpellChecker::IgnoreWords 	extensions/spellcheck/src/mozInlineSpellChecker.cpp:1020
1 	XUL 	JS_ForwardGetElementTo 	js/src/jsobjinlines.h:162
2 	XUL 	nsCxPusher::~nsCxPusher 	objdir-tb/x86_64/mozilla/dist/include/nsCOMPtr.h:676
3 	XUL 	NS_InvokeByIndex 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:162
4 	XUL 	XPCWrappedNative::CallMethod 	js/xpconnect/src/XPCWrappedNative.cpp:2937
5 	libnspr4.dylib 	PR_GetCurrentThread 	nsprpub/pr/src/pthreads/ptthread.c:634
6 	XUL 	nsThreadManager::GetIsMainThread 	xpcom/threads/nsThreadManager.cpp:274
7 	XUL 	NS_IsMainThread 	objdir-tb/x86_64/mozilla/xpcom/build/nsThreadUtils.cpp:137
8 	XUL 	DefinePropertyIfFound 	js/xpconnect/src/xpcprivate.h:485
9 	libnspr4.dylib 	PR_GetCurrentThread 	nsprpub/pr/src/pthreads/ptthread.c:634
10 	XUL 	nsThreadManager::GetIsMainThread 	xpcom/threads/nsThreadManager.cpp:274
11 	XUL 	_ZThn16_N20DeviceStorageRequest14Recv__delete__ERKb 	objdir-tb/x86_64/mozilla/dist/include/nsCOMPtr.h:450
12 	XUL 	XPCCallContext::~XPCCallContext 	js/xpconnect/src/xpcprivate.h:485
13 	XUL 	nsThreadManager::GetCurrentThread 	xpcom/threads/nsThreadManager.cpp:267
14 	XUL 	NS_IsMainThread 	objdir-tb/x86_64/mozilla/xpcom/build/nsThreadUtils.cpp:137
15 	XUL 	_ZThn16_N20DeviceStorageRequest14Recv__delete__ERKb 	objdir-tb/x86_64/mozilla/dist/include/nsCOMPtr.h:450
16 	XUL 	XPC_WN_CallMethod 	js/xpconnect/src/XPCWrappedNativeJSOps.cpp:1480

Running Thunderbird trunk from today. STR :

Reply to an email in english, while default dictonary is set to french (don't know if that matter but that's how I'm currently setup).

Hittinf cmd-r will trigger the crash - not always - though.

Comment 1

[Scoobidiver (away)]

It has been hit by two users in TB 24.0a1/20130506. It's likely a regression from bug 856270.

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozInlineSpellChecker%3A%3AIgnoreWords%28unsigned+short+const**%2C+unsigned+int%29

Comment 2

[Bob Clary [:bc] (inactive)]

bp-26d4f6a8-a203-4ef9-9653-b7ba62130606
reply to english email with default dictionary set to english.
linux x86_64
had previously noted that the spell check had not displayed the words being checked.

Comment 3

[Scoobidiver (away)]

Just in case, here is the regression range:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=8f9ba85eb61c&tochange=a47f4e36197f
http://hg.mozilla.org/comm-central/pushloghtml?fromchange=1d440abb231f&tochange=d031447ec78b

More reports also at:
https://crash-stats.mozilla.com/report/list?signature=mozInlineSpellChecker%3A%3AIgnoreWords%28wchar_t+const**%2C+unsigned+int%29

Comment 4

[Drew Willcoxon :adw]

Attachment: assert mSpellCheck != null

https://tbpl.mozilla.org/?tree=Try&rev=26594cdd6710

IgnoreWords should assert that mSpellCheck is nonnull, like IgnoreWord and related methods do.

I probably dropped the ball in not relating bug 856270's spell checker changes to Thunderbird developers.  According to mxr it hasn't been updated (And why would it have been?  Nobody told them.), which probably explains this:

(In reply to Bob Clary [:bc:] from comment #2)
> had previously noted that the spell check had not displayed the words being
> checked.

And it also explains this crash.  What probably happened is that SetEnableRealTimeSpell was called as usual, but mSpellCheck now remains null when it finishes.  It remains null until nsEditorSpellCheck's async initialization finishes.  IgnoreWords must have been called before then.  (Other explanations would have meant crashes even before bug 856270 landed.)

There's one other site where mSpellCheck is assumed to be nonnull, DoSpellCheck.  DoSpellCheck ends up getting called only by ResumeCheck, which returns early if !mSpellCheck, so an assertion in it shouldn't be necessary.  However, I added one there anyway as a kind of easily seen precondition.

This patch should prevent the crash, but Thunderbird still needs to be updated to account for bug 856270.  I'll file a bug for that.

Comment 5

[Drew Willcoxon :adw]

(In reply to Drew Willcoxon :adw from comment #4)
> This patch should prevent the crash, but Thunderbird still needs to be
> updated to account for bug 856270.  I'll file a bug for that.

bug 880595

Comment 6

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/mozilla-inbound/rev/7266f50150f1

Comment 8

[Ludovic Hirlimann [:Usul]]

(In reply to Drew Willcoxon :adw from comment #6)
> https://hg.mozilla.org/integration/mozilla-inbound/rev/7266f50150f1

is this long enough for a push to trunk ?

Comment 9

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/7266f50150f1

Comment 10

[Wayne Mery (:wsmwk)]

This is gone from Thunderbird crashes starting with daily builds of 2013-06-10

Comment 11

[Paul Silaghi, QA [:pauly]]

No crashes in the crashstats.