Closed Bug 694002 Opened 13 years ago Closed 13 years ago

crash in AffixMgr::~AffixMgr


(Core :: Spelling checker, defect)

Windows 7
Not set





(Reporter: kairo, Assigned: ehsan.akhgari)



(Keywords: crash, Whiteboard: [fixed-in-hunspell-1.3.3])

Crash Data


(1 file, 3 obsolete files)

This bug was filed from the Socorro interface and is 
report bp-baae94d2-bd6a-44d6-90f2-28c922111012 .
============================================================= 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:

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

100% (264/264) vs.   1% (511/92174) (Canadian English Dictionary,
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."

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?
Crash Signature: [@ AffixMgr::~AffixMgr()] → [@ AffixMgr::~AffixMgr()] [@ SfxEntry::~SfxEntry() ]
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.
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.)

Version 2.0.3 has just been approved.
Thanks for the rapid review, Jorge.

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

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?
Crash Signature: [@ AffixMgr::~AffixMgr()] [@ SfxEntry::~SfxEntry() ] → [@ AffixMgr::~AffixMgr()] [@ SfxEntry::~SfxEntry() ] [@ AffixMgr::~AffixMgr ]
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
Attached patch initialize sfx the same as pfx (obsolete) — Splinter Review
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?
yeah, fix checked into upstream hunspell. As is an extra fix for a leak with the same slightly busted .aff
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.)
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.
Crash Signature: [@ AffixMgr::~AffixMgr()] [@ SfxEntry::~SfxEntry() ] [@ AffixMgr::~AffixMgr ] → [@ AffixMgr::~AffixMgr()] [@ SfxEntry::~SfxEntry() ] [@ AffixMgr::~AffixMgr ] [@ arena_dalloc | free | SfxEntry::~SfxEntry() ]
Attached patch Mozilla patch (obsolete) — Splinter Review
Thanks a lot Caolan for the patch!
Assignee: nobody → ehsan
Attachment #566911 - Flags: review?(caolanm)
Whiteboard: [fixed-in-hunspell-1.3.3]
Drive-by nit - please update README.hunspell too.
Attached patch Mozilla patch (obsolete) — Splinter Review
Attachment #566911 - Attachment is obsolete: true
Attachment #566911 - Flags: review?(caolanm)
Attachment #567299 - Flags: review?(caolanm)
Attachment #567299 - Flags: review?(caolanm) → review?(ryanvm)
Attachment #567299 - Flags: review?(ryanvm) → review?(bugs)
Comment on attachment 567299 [details] [diff] [review]
Mozilla patch

Attachment #567299 - Flags: review?(bugs) → review+
Ehsan, looks like this never landed?
If so, may bug 710940 be related? Also, can we land this for 11 at least, please?
Ehsan, ping^2
Attached patch PatchSplinter Review
The attached patch actually bitrotted slightly thanks to bug 710967.
Attachment #566773 - Attachment is obsolete: true
Attachment #567299 - Attachment is obsolete: true
Attachment #584257 - Flags: review+
Landed fix on m-i:
(updating branch to "Trunk" because until now, this affected trunk, and that's where the fix is landing for now at least)
Keywords: checkin-needed
Target Milestone: --- → mozilla12
Version: 9 Branch → Trunk
Closed: 13 years ago
Resolution: --- → FIXED
Sorry for dropping the ball here, and thanks for landing this, Daniel.
You need to log in before you can comment on or make changes to this bug.