Closed Bug 100522 Opened 23 years ago Closed 2 years ago

TCP connection to news server closed unnecessarily

Categories

(MailNews Core :: Networking: NNTP, defect)

x86
All
defect
Not set
minor

Tracking

(thunderbird_esr91 wontfix)

RESOLVED FIXED
96 Branch
Tracking Status
thunderbird_esr91 --- wontfix

People

(Reporter: philanderton, Unassigned)

References

Details

(Whiteboard: [fixed by bug 1707550])

Attachments

(2 files)

This bug is a follow-up to my question about
nsNntpIncomingServer::ConnectionTimeOut in bug 59449. I asked why we seem to
close a connection to the news server after 170 seconds of inactivity, and Seth
Spitzer answered:
> we do that because if you haven't used the connection in 170 seconds, we assume
> it's not longer active because the server dropped us.

Well, I've looked at this a bit more closely - this functions get called every
time we request a new article. If more than 170 seconds have elapsed since we
last did this, we send a QUIT to the news server and take down the TCP
connection. Then of course, we build up a new connection, maybe authenticate
again, select the GROUP again, before we finally request the article.

Why? I've tried sitting and waiting, and I see that news.mozilla.org waits 1500
seconds before disconnecting. Another news server did so after 600 seconds. In
both cases they send a 503 response first, so I am sure we would notice when a
server drops us.

IMHO this code is unnecessary.
I'm going to be deliberately provocative and propose a patch that completely
removes nsNntpIncomingServer::ConnectionTimeOut. I've had this in my local tree
for some time with no adverse effects.
Keywords: patch
This sounds like an interesting fix, but I'm worried that we'll end up 
completely horked if the connection indeed *does* time out.

Phil, is there any chance you can deliberately time-out some connection to a 
news server, and see what happens with your patch applied?
Håkan, that is exactly what I did before filing this bug. ISTR the server sends
a 503 message saying something like "connection timed out", and then closes the
TCP connection. I'll try to make a little NNTP trace on Monday to show this.
Comment on attachment 50298 [details] [diff] [review]
Patch to remove nsNntpIncomingServer::ConnectionTimeOut

Ran with the patch for a while and pondered its effects.

r=hwaara
Attachment #50298 - Flags: review+
I will land this for you Phil, once sspitzer has super-reviewed it.
Assignee: sspitzer → philanderton
Keywords: approval
Status: NEW → ASSIGNED
zzzzz...
Seth?
Phil, you should send your sr= request(s) to sspitzer directly via email,
following the guidelines set out by mozilla.org (see the super-reviewer page),
and CC reviewers@mozilla.org with your email.
Cancelled due to lack of interest.
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
OS: Windows 98 → All
Resolution: --- → WONTFIX
re-opening, as it sounds valid.  

news doesn't get much love, so news bugs tend to sit in bugzilla for a while.

but we do get to them eventuall.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
taking, but to target milestone for this yet.
Assignee: philanderton → sspitzer
Status: REOPENED → NEW
Product: MailNews → Core
Is this patch still necessary?  Does it still apply?
Assignee: sspitzer → nobody
QA Contact: stephend
Yes, but see comment #12. I think it's clear that no-one cares about news any more
Assignee: nobody → sspitzer
sorry, not going to work on this.
Assignee: sspitzer → nobody
Joshua, has patch and review. What do you think?
QA Contact: networking.news
Comment on attachment 50298 [details] [diff] [review]
Patch to remove nsNntpIncomingServer::ConnectionTimeOut

RFC 3977 has since been published, and it is a little bit more clear. Quoting from RFC 3977:
   An NNTP server MAY have an inactivity autologout timer.
   [ ... ]
                                When the timer expires, the server
   SHOULD close the connection without sending any response to the
   client.

Although it appears that common newsservers ignore this requirement (fakeserver does not), the patch does not appear to rely on a response code to work.
Attachment #50298 - Flags: superreview?(bienvenu)
Attachment #50298 - Flags: superreview?(bienvenu) → superreview+
Comment on attachment 50298 [details] [diff] [review]
Patch to remove nsNntpIncomingServer::ConnectionTimeOut

if jcranmer's still ok with this, we can try it out.
Product: Core → MailNews Core
Joshua, want to take and drive this in for beta 2?

Still an issue?

Flags: needinfo?(remotenonsense)

Since bug 1707550, we wait for timeout error and server close event now.

Status: NEW → RESOLVED
Closed: 22 years ago2 years ago
Flags: needinfo?(remotenonsense)
Resolution: --- → FIXED
Whiteboard: [fixed by bug 1707550]
Target Milestone: --- → 96 Branch
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: