Last Comment Bug 694002 - crash in AffixMgr::~AffixMgr
: crash in AffixMgr::~AffixMgr
Status: RESOLVED FIXED
[fixed-in-hunspell-1.3.3]
: crash
Product: Core
Classification: Components
Component: Spelling checker (show other bugs)
: Trunk
: x86_64 Windows 7
: -- critical (vote)
: mozilla12
Assigned To: :Ehsan Akhgari (busy, don't ask for review please)
:
Mentors:
Depends on: hunspell-1.3.3
Blocks:
  Show dependency treegraph
 
Reported: 2011-10-12 07:27 PDT by Robert Kaiser (not working on stability any more)
Modified: 2014-06-16 06:42 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
initialize sfx the same as pfx (630 bytes, patch)
2011-10-13 03:13 PDT, Caolan McNamara
no flags Details | Diff | Review
Mozilla patch (973 bytes, patch)
2011-10-13 12:47 PDT, :Ehsan Akhgari (busy, don't ask for review please)
no flags Details | Diff | Review
Mozilla patch (1.87 KB, patch)
2011-10-15 13:39 PDT, :Ehsan Akhgari (busy, don't ask for review please)
bugs: review+
Details | Diff | Review
Patch (1.93 KB, patch)
2011-12-25 06:11 PST, Ryan VanderMeulen [:RyanVM]
ryanvm: review+
Details | Diff | Review

Description Robert Kaiser (not working on stability any more) 2011-10-12 07:27:22 PDT
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) en-CA@dictionaries.addons.mozilla.org (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 2011-10-12 07:42:01 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2011-10-12 08:27:49 PDT
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] 2011-10-12 10:03:24 PDT
Version 2.0.3 has just been approved.
Comment 4 Paul Schmiedge 2011-10-12 11:18:06 PDT
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 2011-10-13 03:01:08 PDT
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 2011-10-13 03:13:09 PDT
Created attachment 566773 [details] [diff] [review]
initialize sfx the same as pfx
Comment 7 Robert Kaiser (not working on stability any more) 2011-10-13 05:42:03 PDT
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 2011-10-13 07:02:17 PDT
yeah, fix checked into upstream hunspell. As is an extra fix for a leak with the same slightly busted .aff
Comment 9 Robert Kaiser (not working on stability any more) 2011-10-13 07:07:50 PDT
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 2011-10-13 07:15:15 PDT
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 :Ehsan Akhgari (busy, don't ask for review please) 2011-10-13 12:47:38 PDT
Created attachment 566911 [details] [diff] [review]
Mozilla patch

Thanks a lot Caolan for the patch!
Comment 12 Ryan VanderMeulen [:RyanVM] 2011-10-15 05:40:31 PDT
Drive-by nit - please update README.hunspell too.
Comment 13 :Ehsan Akhgari (busy, don't ask for review please) 2011-10-15 13:39:53 PDT
Created attachment 567299 [details] [diff] [review]
Mozilla patch
Comment 14 Olli Pettay [:smaug] 2011-11-30 03:34:34 PST
Comment on attachment 567299 [details] [diff] [review]
Mozilla patch

rs=e
Comment 15 Ryan VanderMeulen [:RyanVM] 2011-12-16 12:10:59 PST
Ehsan, looks like this never landed?
Comment 16 Robert Kaiser (not working on stability any more) 2011-12-16 12:35:59 PST
If so, may bug 710940 be related? Also, can we land this for 11 at least, please?
Comment 17 Ryan VanderMeulen [:RyanVM] 2011-12-22 07:42:11 PST
Ehsan, ping^2
Comment 18 Ryan VanderMeulen [:RyanVM] 2011-12-25 06:11:10 PST
Created attachment 584257 [details] [diff] [review]
Patch

The attached patch actually bitrotted slightly thanks to bug 710967.
Comment 19 Daniel Holbert [:dholbert] 2011-12-27 14:13:38 PST
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) 2011-12-28 11:08:59 PST
https://hg.mozilla.org/mozilla-central/rev/e832c81d1214
Comment 21 :Ehsan Akhgari (busy, don't ask for review please) 2011-12-29 15:46:32 PST
Sorry for dropping the ball here, and thanks for landing this, Daniel.

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