Closed Bug 670935 Opened 9 years ago Closed 8 years ago

News crash [@ nsMsgKeySet::Output(char**) ] | [@ nsMsgNewsFolder::UpdateSummaryFromNNTPInfo ] and [@ nsMsgKeySet::Output ] (Mac)

Categories

(MailNews Core :: Backend, defect, critical)

x86
All
defect
Not set
critical

Tracking

(thunderbird9 fixed, thunderbird10 fixed)

RESOLVED FIXED
Thunderbird 11.0
Tracking Status
thunderbird9 --- fixed
thunderbird10 --- fixed

People

(Reporter: wsmwk, Assigned: jcranmer)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

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?
Depends on: 662228
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.
Duplicate of this bug: 672603
Duplicate of this bug: 694522
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.
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
Attachment #576066 - Flags: review?(dbienvenu)
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 ;-)
Attachment #576066 - Flags: review?(dbienvenu) → review+
This has been pushed, with the comment changed. I think I meant to change the name originally, but forgot when I uploaded the patch.
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Duplicate of this bug: 706002
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.
Attachment #576066 - Flags: approval-comm-beta?
Attachment #576066 - Flags: approval-comm-aurora?
Attachment #576066 - Flags: approval-comm-beta?
Attachment #576066 - Flags: approval-comm-beta+
Attachment #576066 - Flags: approval-comm-aurora?
Attachment #576066 - Flags: approval-comm-aurora+
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).
Flags: in-testsuite+
Target Milestone: --- → Thunderbird 11.0
You need to log in before you can comment on or make changes to this bug.