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

RESOLVED FIXED in Thunderbird 11.0

Status

MailNews Core
Backend
--
critical
RESOLVED FIXED
6 years ago
6 years ago

People

(Reporter: wsmwk, Assigned: jcranmer)

Tracking

({crash})

Trunk
Thunderbird 11.0
x86
All
crash
Bug Flags:
in-testsuite +

Thunderbird Tracking Flags

(thunderbird9 fixed, thunderbird10 fixed)

Details

(crash signature)

Attachments

(1 attachment)

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
(Assignee)

Comment 1

6 years ago
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
(Assignee)

Comment 3

6 years ago
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.
(Assignee)

Updated

6 years ago
Duplicate of this bug: 672603
(Assignee)

Updated

6 years ago
Duplicate of this bug: 694522
(Assignee)

Comment 6

6 years ago
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.
Assignee: nobody → Pidgeot18
Status: NEW → ASSIGNED
Attachment #576066 - Flags: review?(dbienvenu)

Comment 7

6 years ago
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 8

6 years ago
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+
(Assignee)

Comment 9

6 years ago
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
Last Resolved: 6 years ago
Resolution: --- → FIXED
(Assignee)

Updated

6 years ago
Duplicate of this bug: 706002
(Assignee)

Comment 11

6 years ago
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).
status-thunderbird10: --- → fixed
status-thunderbird9: --- → fixed
Flags: in-testsuite+
Target Milestone: --- → Thunderbird 11.0
You need to log in before you can comment on or make changes to this bug.