506669 - crash [@ NS_strlen(unsigned short const*)] - [@ nsImapProtocol::ShowProgress] with IMAP and Galician locale.

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Wayne Mery (:wsmwk)]

(pulled from crash-stats)
crash [@ NS_strlen(unsigned short const*)] with Exchange messages - 3 comments, all mention working with Exchange

Only appearing in 3.0b3 (not b3pre or b4pre) - so might be a regression.
Need steps to reproduce, so => qawanted

bp-689e1f94-55ee-4e42-ad96-314272090722
0  	xpcom_core.dll  	NS_strlen  	 nsCRTGlue.cpp:102
1 	xpcom_core.dll 	cvt_S 	nsTextFormatter.cpp:544
2 	xpcom_core.dll 	dosprintf 	nsTextFormatter.cpp:1136
3 	xpcom_core.dll 	nsTextFormatter::vsmprintf 	nsTextFormatter.cpp:1287
4 	xpcom_core.dll 	nsTextFormatter::smprintf 	nsTextFormatter.cpp:1248
5 	thunderbird.exe 	nsImapProtocol::ShowProgress 	mailnews/imap/src/nsImapProtocol.cpp:4954
6 	thunderbird.exe 	nsImapServerResponseParser::msg_fetch 	mailnews/imap/src/nsImapServerResponseParser.cpp:1071
7 	thunderbird.exe 	nsImapServerResponseParser::response_data 	mailnews/imap/src/nsImapServerResponseParser.cpp:756
8 	thunderbird.exe 	nsImapServerResponseParser::ParseIMAPServerResponse 	mailnews/imap/src/nsImapServerResponseParser.cpp:243
9 	thunderbird.exe 	nsImapProtocol::ParseIMAPandCheckForNewMail 	mailnews/imap/src/nsImapProtocol.cpp:1853
10 	thunderbird.exe 	nsImapProtocol::FetchMessage 	mailnews/imap/src/nsImapProtocol.cpp:3416
11 	thunderbird.exe 	nsImapProtocol::FolderMsgDumpLoop 	mailnews/imap/src/nsImapProtocol.cpp:4154
12 	thunderbird.exe 	nsImapProtocol::FolderMsgDump 	mailnews/imap/src/nsImapProtocol.cpp:4058
13 	thunderbird.exe 	nsImapProtocol::FolderHeaderDump 	mailnews/imap/src/nsImapProtocol.cpp:4038
14 	thunderbird.exe 	nsImapProtocol::ProcessMailboxUpdate 	mailnews/imap/src/nsImapProtocol.cpp:4005
15 	thunderbird.exe 	nsImapProtocol::ProcessSelectedStateURL 	mailnews/imap/src/nsImapProtocol.cpp:2819
16 	thunderbird.exe 	nsImapProtocol::ProcessCurrentURL 	mailnews/imap/src/nsImapProtocol.cpp:1709
17 	thunderbird.exe 	nsImapProtocol::ImapThreadMainLoop 	mailnews/imap/src/nsImapProtocol.cpp:1360
18 	thunderbird.exe 	nsImapProtocol::Run 	mailnews/imap/src/nsImapProtocol.cpp:1059
19 	xpcom_core.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
20 	xpcom_core.dll 	NS_ProcessNextEvent_P 	nsThreadUtils.cpp:227
21 	xpcom_core.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:254
22 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:426
23 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:122
24 	mozcrt19.dll 	_callthreadstartex 	threadex.c:348
25 	mozcrt19.dll 	_threadstartex 	threadex.c:326

Comment 1

[David :Bienvenu]

can we tell if those are en-US builds or other locales?

Comment 2

[Ludovic Hirlimann [:Usul]]

(In reply to comment #1)
> can we tell if those are en-US builds or other locales?

Not really :-(

Comment 3

[Wayne Mery (:wsmwk)]

#5 ranking for 3.0, ~2% of crashes (using 3 day period instead of 28)
_however_, some, like the unix ones, are probably from one user
http://crash-stats.mozilla.com/report/list?range_value=2&range_unit=weeks&signature=NS_strlen%28unsigned%20short%20const*%29&version=Thunderbird%3A3.0

on recheck of crash-stats the earliest I find is 3.0b2pre 20090216035229.
but nothing with comments :(

looks like startup crash.
playing cautious and requesting blocking for fixing in 3.0.something

Comment 4

[Mark Banner (:standard8)]

This isn't a huge crasher as its hanging around 20-something in the crash stacks. For now, I'm going to say that this is needed but not blocking .1.

Comment 6

[timeless]

82% (9/11) vs.   0% (10/6235) [e-mail]
36% (4/11) vs.   0% (9/6235) [e-mail]

seems fairly common, but we now have a live reporter (IagoSRL), so please feel free to ask him questions :).

Comment 7

[IagoSRL]

I did some test and I think my problem is in my locale 'gl-ES' installation. I downloaded and tried the version 3.0.2 gl-ES and the problem persist, then I downloaded the en-US and es-ES versions and tried with the same profile: no crash, no bug.

Before this, I tested with beta versions, and all works fine:
 - nightly build: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2.2pre) Gecko/20100226 Lanikai/3.1b1pre
 - 3.1 alpha 1: Mozilla/5.0 (Windows; U; Windows NT 5.1; es-ES; rv:1.9.2pre) Gecko/20100119 Lanikai/3.1a1

The crash is not at startup, but when TB is getting messages from Gmail IMAP (maybe another IMAP too, I don't know).
With Gmail POP don't crash.

I will use es-ES version from now and report any problem -related with this bug- that might occur.

Question: How/Why a 'locale' may cause a crash?

Thanks.

Comment 8

[Mark Banner (:standard8)]

So I've figured out the issue, and the primary issue is indeed an issue with the 'gl' (Galician), although its not helped by unclear localisation comments.

I've checked though the other locales and they don't have this problem.

The primary issue is that gl has two strings:

5036=Descargando %lu de %lu cabeceiras de mensaxes de %S
5037=Descargando %lu de %lu marcas de mensaxes de %S

however, en-US has these as:

5036=%S Downloading message header %lu of %lu
5037=%S Downloading message flag %lu of %lu

So what ends up happening is that in the gl case, we're trying to smprintf a string to %lu and it just doesn't know what to do and crashes :-(

The solution for gl would be to change to number based parameters, i.e:

5036=Descargando %2$lu de %3$lu cabeceiras de mensaxes de %1$S

However, I'd also like to at least fix the localisation notes so that new locales or updates to them aren't affected. This can be done in 3.0.

From a plurals perspective I'm not sure if the strings are right, but if someone wants to improve those I suggest we cover them in a different bug.

Comment 9

[Mark Banner (:standard8)]

Attachment: Fix locale comment, don't touch string

Like I said, fix the locale comment, don't touch the string. We can then put this in on the 3.0 branch.

I'm filing a separate bug for fixing gl.

If we want to fix the strings to be much better, we should do that in another bug.

Comment 10

[Mark Banner (:standard8)]

Attachment: Fix locale comment, don't touch string v2

Ok, I just realised that the previous thing I'd attached was the file and not the patch.

So here's the proper patch that I was proposing for correcting the comments.

Comment 11

[Simon Paquet [:sipaq]]

Comment on attachment 431970 [details] [diff] [review]
Fix locale comment, don't touch string v2

r=sipaq

Can you please post a message to mozilla.dev.l10n when you've committed this to inform localizers about this change. Normally they do not notice comment changes. 

Thanks!

Comment 12

[Mark Banner (:standard8)]

Comment on attachment 431970 [details] [diff] [review]
Fix locale comment, don't touch string v2

a=Standard8 as this is comment-only patch.

Comment 13

[Mark Banner (:standard8)]

Checked into trunk and branch:

http://hg.mozilla.org/comm-central/rev/96b067a9e19e
http://hg.mozilla.org/releases/comm-1.9.1/rev/b9a1561a34a0

Comment 14

[Mark Banner (:standard8)]

Posted to m.d.l10n and raised bug 551919 on replacing the number identifiers with string versions (which should also ensure some extra tidy up).

Marking as fixed for trunk as we've fixed the comments - Bug 549442 will fix the broken locale. Leaving as not fixed for 3.0.4 so that I have a reference to remember to ensure that bug 549442 is fixed for the release.

Comment 15

[Mark Banner (:standard8)]

New gl nightlies are now out and seem to be working fine, hence marking this as fixed for 3.0.4.