News crash [@ nsMsgKeySet::Output(char**) ] | [@ nsMsgNewsFolder::UpdateSummaryFromNNTPInfo ] spun off from bug 188243 bp-284b8e61-2f00-46a4-ae2b-dfb7b2110710 can't support chinese newsgroup bp-18667bce-acea-47a3-b972-577992110710 cannot read email since the last update [to version 5]. up until now. it's alway crash. bp-60e19aae-c370-4323-b8c0-f5e422110630 (osd) bp-9dfe2c9e-9c25-4837-a86f-c28bc2110705 (chris) a single NNTP server added and as soon as I expand the list of newsgroups on this server (using the left panel that shows "Local Folders" and my NNT Server), within seconds, Thunderbird crashes. Happens every time since I upgraded to Thunderbird 5.0. Worked fine before the upgrade from previous version. EXCEPTION_ACCESS_VIOLATION_READ 0x8 0 xul.dll nsMsgKeySet::Output mailnews/base/util/nsMsgKeySet.cpp:315 1 xul.dll nsMsgNewsFolder::UpdateSummaryFromNNTPInfo mailnews/news/src/nsNewsFolder.cpp:735 2 xul.dll nsNNTPProtocol::BeginReadXover mailnews/news/src/nsNNTPProtocol.cpp:3118 3 xul.dll nsNNTPProtocol::ProcessProtocolState mailnews/news/src/nsNNTPProtocol.cpp:4659 4 xul.dll nsMsgProtocol::OnDataAvailable mailnews/base/util/nsMsgProtocol.cpp:387 5 xul.dll nsInputStreamPump::OnStateTransfer netwerk/base/src/nsInputStreamPump.cpp:510 6 xul.dll nsInputStreamPump::OnInputStreamReady netwerk/base/src/nsInputStreamPump.cpp:400
Hmm, we're getting a null nsMsgKeySet. It looks like the mReadSet is initially set by AddNewsgroup (effectively) fairly early on, so I doubt that it's an initialization failure. More likely, someone is triggering us to delete the mReadSet before we read from it again. Doing some more research.
(In reply to Joshua Cranmer [:jcranmer] from comment #1) > More likely, someone is triggering us to delete the mReadSet before we read > from it again. Doing some more research. Joshua, did the research bear fruit?
Research yielding fruit: So, this is a combination of RDF, URIs, and characters needing escapes. We create two folders, one with an escaped name and one without. One of these gets initialized; the other one does not. We're crashing because we end up using the uninitialized folder. If you will excuse me, I need to bash something with a bloody hammer.
Created attachment 576066 [details] [diff] [review] Make FindGroup use escaped names Fortunately, all of the news URI work means there are very few places where I need to change this to fix the problems caused by RDF's ability to conjure up nonexistent folder names if they aren't sufficiently well-escaped. Side rant of the day: if you have dependent strings in use, you can end up with wrong results.
Comment on attachment 576066 [details] [diff] [review] Make FindGroup use escaped names Thx for the patch; I'll build and try it now. Can you explain why (and what) is necessary? I assume it's the temp var, and the reason is that you can't unescape into the (sub) string you're unescaping from. If that's the case, please just say // Can't unescape into the string we're unescaping from. or something like that. + // Yes, this is necessary + nsCAutoString unescapedGroup; + MsgUnescapeString(Substring(group, groupSeparator + 1), 0, unescapedGroup); + group = unescapedGroup;
Comment on attachment 576066 [details] [diff] [review] Make FindGroup use escaped names r=me, modulo my comment about your obscure comment ;-)
This has been pushed, with the comment changed. I think I meant to change the name originally, but forgot when I uploaded the patch.
Comment on attachment 576066 [details] [diff] [review] Make FindGroup use escaped names Since this crash was introduced in Version 8, I'd like to request that this gets uplifted to aurora and beta.
Please remember to include the changeset details in bugs when landing patches. This makes it easier to find them, or transplant them if we're looking at bugs in future. The original landing was: http://hg.mozilla.org/comm-central/rev/2d85ecaf36c6 I've transplanted this to aurora & beta, as we are building a new beta today: http://hg.mozilla.org/releases/comm-aurora/rev/8ac2cbe36ec1 http://hg.mozilla.org/releases/comm-beta/rev/0146a1f973e2 (oh and on the idl front, I'm pretty sure the attribute name change doesn't affect binary extensions - due to not being a critical part of the function definition - which is why I've allowed it).