The default bug view has changed. See this FAQ.

Crash when trying to download a newsgroup via the context menu

RESOLVED FIXED in Thunderbird 55.0

Status

MailNews Core
Networking: NNTP
RESOLVED FIXED
15 days ago
10 days ago

People

(Reporter: Alfred Peters, Assigned: Jorg K (GMT+1))

Tracking

({regression})

Trunk
Thunderbird 55.0
regression

Thunderbird Tracking Flags

(thunderbird53 fixed, thunderbird54 fixed, thunderbird55 fixed)

Details

Attachments

(1 attachment)

(Reporter)

Description

15 days ago
User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:54.0) Gecko/20100101 Firefox/54.0
Build ID: 20170301192304

Steps to reproduce:

Select a Newsgroup
open the context menu (RMC)
Properties -> /Synchronization\
 => [Download Now]


Actual results:

TB crashes

It does not matter if the checkbox ('Select this newsgroup for offline use') is ticked.
(Reporter)

Comment 1

15 days ago
Comm-Central:
last good Changeset:
21218 (5f72602f1ff0) Bug 1337865 - Force release of subserver's listener. r=aceman

first bat Changeset:
21219 (ef7420896e68) Bug 1337865 - fix some leaking objects in mailnews/news. r=aceman
(Reporter)

Updated

15 days ago
Blocks: 1337865
Keywords: regression
(Assignee)

Comment 2

15 days ago
Created attachment 8846080 [details] [diff] [review]
1346338-fix-crash.patch

Thanks for reporting this and thanks for being on the Daily/Aurora channel where the ride can be bumpy, as we see here.

Aceman, please keep in mind that 'new' is infallible in Mozilla, if the memory cannot be allocated, control doesn't return to the caller.
Assignee: nobody → jorgk
Status: UNCONFIRMED → ASSIGNED
Ever confirmed: true
Attachment #8846080 - Flags: review?(acelists)
(Assignee)

Updated

15 days ago
Component: Untriaged → Networking: NNTP
OS: Unspecified → All
Product: Thunderbird → MailNews Core
Hardware: Unspecified → All

Comment 3

15 days ago
There is no crash report in this bug. So how does the fix work? How did we cause this in bug 1337865 ?

Comment 4

15 days ago
And how is RefPtr better than the original code?
(Assignee)

Comment 5

15 days ago
Oh, I thought this was a simple review :-(

OK, no crash report, but the crash is easily reproducible, just follow the steps.
How did we cause this. Easy. There was a new object allocated, which was clearly leaking. So I added a 'delete' in the function that created it. Obviously though, someone else grabbed a reference to the object which was dereferenced later/elsewhere/asynchronously/who-knows.

So the obvious Mozilla fix is letting ref-counting do its magic. The object will magically die when the last interested party releases the reference.

Comment 6

15 days ago
Comment on attachment 8846080 [details] [diff] [review]
1346338-fix-crash.patch

Review of attachment 8846080 [details] [diff] [review]:
-----------------------------------------------------------------

OK, let's try it. But I do not see how can somebody outside keep reference to downloadState .
Attachment #8846080 - Flags: review?(acelists) → review+
(Assignee)

Comment 7

15 days ago
It's invisible, like all the workings of software. But seriously:

downloadState->DownloadArticles(...) calls nsNewsDownloader::DownloadArticles(). In there we set a bunch of member variables, call DownloadNext(true) and return. Upon return the object is destroyed, way before the async download is finished. Surely I should have know this before causing the crash ;-(
(Assignee)

Comment 8

15 days ago
https://hg.mozilla.org/comm-central/rev/a10e7c19d7f373693e2a193204f17b4de3f7f741
Status: ASSIGNED → RESOLVED
Last Resolved: 15 days ago
status-thunderbird54: --- → affected
status-thunderbird55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 55.0
(Assignee)

Comment 9

15 days ago
Comment on attachment 8846080 [details] [diff] [review]
1346338-fix-crash.patch

We'll need to fix the crash in Aurora as well.

This will go to ESR 52 together with bug 1337865 in due course.
Attachment #8846080 - Flags: approval-comm-esr52?
Attachment #8846080 - Flags: approval-comm-aurora+
(Assignee)

Comment 10

15 days ago
Aurora (TB 54):
https://hg.mozilla.org/releases/comm-aurora/rev/39eaa29525624f85ed1bc0fdb7e71be54d4c2b06

Crash will be fixed in Earlybird and Daily tomorrow. Thanks again for reporting it.
status-thunderbird54: affected → fixed
(Assignee)

Updated

15 days ago
status-thunderbird53: --- → affected
(Assignee)

Updated

15 days ago
Attachment #8846080 - Flags: approval-comm-beta+
(Assignee)

Comment 11

10 days ago
Beta (TB 53):
https://hg.mozilla.org/releases/comm-beta/rev/27fd17b01040de8023be9ec3ef4b979d4939b16d
status-thunderbird53: affected → fixed
You need to log in before you can comment on or make changes to this bug.