694002 - crash in AffixMgr::~AffixMgr

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-baae94d2-bd6a-44d6-90f2-28c922111012 .
============================================================= 

https://crash-stats.mozilla.com/report/list?signature=AffixMgr%3A%3A~AffixMgr%28%29 for more reports - this abruptly started happening yesterday in Firefox and Thunderbird 7.0.1 mostly, but other versions (both older and newer) are indicated as well. Seems to be pretty exclusive to Vista and Win7 users, the address of the crash is always 0x1010438 on 32bit processors, seems to be 0x4a01010458 on 64bit.

Comments indicate that this happens at shutdown, when changing dictionaries, or sometimes even when they are typing and the spllechecker is running.


Correlations for Firefox 7.0.1 Windows NT:

Modules:
96% (253/264) vs.  29% (26615/92174) EhStorShell.dll
(all other listed modules look pretty normal)

Add-ons:
100% (264/264) vs.   1% (511/92174) [e-mail] (Canadian English Dictionary, https://addons.mozilla.org/addon/3653)
4% (10/264) vs.   0% (58/92174) 2.0.1
96% (254/264) vs.   0% (453/92174) 2.0.2


It looks like the dictionary itself had some problem with the affix file, I wonder if we really should be allowed to crash for that, though.

The author has removed version 2.0.2 from AMO and has uploaded a not-yet-reviewed 2.0.3 that states:
"Version 2.0.3 Released October 11, 2011 194.6 KB Works with Firefox 2.0 - 10.0a1, Thunderbird 2.0 - 10.0a1, SeaMonkey 2.0 - 2.7a1
This update addresses multiple critical bugs that were introduced in the affix file in 2.0.2, including one that caused Firefox to crash. Please update to this version."
See https://addons.mozilla.org/en-US/firefox/addon/canadian-english-dictionary/versions/?page=1#version-2.0.3

We also have some crashes with the 2.0.1 version, though, and I wonder if there's a generic code problem that actually causes the crash.


I've CCed Paul Schiedge, the author of the add-on, hopefully he can also shed some light on the actual problem.

Jorge, can we get the 2.0.3 version reviewed fast so people get updated off the crashy version?

Comment 1

[Paul Schmiedge]

Thanks for the CC.  The reason the crash started yesterday is because the 2.0.2 version of the addon passed review and started trickling out to users.  I got emails complaining of the problem fairly quickly after that.

The problem with my addon was that I had added a new line to the affix file - a "REP" statement (valid replacement for spelling suggestions) - but did not update the counter line that informs the engine how many statements to expect.

The fix was simply to adjust the first "REP" declaration to match the actual number of "REP" declarations in the file.

I missed it in testing because I did my test on a Mac (OSX 10.6, FF 7.0), but it turns out that the declaration mismatch caused lots of other problems besides crashing - some suffix/prefix declarations were no longer parsing, so people were seeing lots of additional spelling errors, too.

No one has previously brought my attention to crashes related to prior versions of the addon.

Comment 2

[(no longer active)]

Paul, thanks a lot for your comment!

I would really like to fix this crash anyway, since Firefox shouldn't crash when a dictionary add-on is installed anyways!  ;-)  I'd like to install the 2.0.2 version of your add-on locally in order to reproduce the crash.  Are you aware of steps that would reliably result in the crash happening?  (I need those steps in order to debug this more effectively.)

Thanks!

Comment 3

[Jorge Villalobos [:jorgev] (he/him)]

Version 2.0.3 has just been approved.

Comment 4

[Paul Schmiedge]

Thanks for the rapid review, Jorge.

Ehsan:
I was able to reliable reproduce the problem simply by finding a page with a text area and typing text with errors.  With the corrupt affix file, it's not hard to produce text with errors.  A convenient test site is http://textarea.org

1. Type text into the text area.  Some sample words that shouldn't error
   (but did):
   "words", "added" "tenses" ,"earlier" , "it's", "its", "holds", "seems",
   "specialists" "there's", "updates" 
2. Close the browser window.
3. Crash report.

I don't have access to my older builds right now (my build machine isn't remotely accessible), but do you need me to provide a copy of the 2.0.2 XPI?

Comment 5

[Caolan McNamara]

reproducible with hunspell master with..

echo "words", "added" "tenses" ,"earlier" , "it's", "its", "holds", "seems", "specialists" "there's", "updates" | valgrind ./tools/.libs/hunspell -d ~/fc/dictionaries/en-CA

from the 2.0.2 dict

Comment 6

[Caolan McNamara]

Attachment: initialize sfx the same as pfx

Comment 7

[Robert Kaiser]

Comment on attachment 566773 [details] [diff] [review]
initialize sfx the same as pfx

I guess you'll also upstream the fix to mainline HunSpell, right?

Comment 8

[Caolan McNamara]

yeah, fix checked into upstream hunspell. As is an extra fix for a leak with the same slightly busted .aff

Comment 9

[Robert Kaiser]

Cool, thanks, you rock!

Oh, you should request review from :ehsan here. (And I guess we then might want to request this to be approved for Aurora and Beta as well.)

Comment 10

[Caolan McNamara]

The attached patch is against hunspell head, rather than against the embedded mozilla one, so "someone else"(tm) needs to tweak it to apply against the in-tree location.

Comment 11

[(no longer active)]

Attachment: Mozilla patch

Thanks a lot Caolan for the patch!

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Drive-by nit - please update README.hunspell too.

Comment 13

[(no longer active)]

Attachment: Mozilla patch

Comment 14

[Olli Pettay [:smaug][[e-mail]]]

Comment on attachment 567299 [details] [diff] [review]
Mozilla patch

rs=e

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Ehsan, looks like this never landed?

Comment 16

[Robert Kaiser]

If so, may bug 710940 be related? Also, can we land this for 11 at least, please?

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Ehsan, ping^2

Comment 18

[Ryan VanderMeulen [:RyanVM]]

Attachment: Patch

The attached patch actually bitrotted slightly thanks to bug 710967.

Comment 19

[Daniel Holbert [:dholbert]]

Landed fix on m-i: https://hg.mozilla.org/integration/mozilla-inbound/rev/e832c81d1214
(updating branch to "Trunk" because until now, this affected trunk, and that's where the fix is landing for now at least)

Comment 20

[Matt Brubeck (:mbrubeck)]

https://hg.mozilla.org/mozilla-central/rev/e832c81d1214

Comment 21

[(no longer active)]

Sorry for dropping the ball here, and thanks for landing this, Daniel.