INN commit: branches/2.4/innfeed (connection.c)
INN Commit
Russ_Allbery at isc.org
Wed Apr 9 18:05:59 UTC 2008
Date: Wednesday, April 9, 2008 @ 11:05:59
Author: iulius
Revision: 7753
In all of the response handlers that idle the connection if there's nothing
left in the queue, don't idle if there are writes pending. The cases where
this could possibly trigger are obscure and involve the remote peer doing
evil things, but the rest of the code handles it correctly and we were still
seeing assertion failures, indicating that evil may be happening.
In issueStreamingCommands, make certain that there are no pending writes
before idling the connection.
Add the code to ihaveBodyDone that was already in commandWriteDone to idle
the connection if the queue is empty in case we'd had to defer the idle in
the response handler due to an unfinished write.
Whenever doSomeWrites is called with writes still pending, add a work
callback to do the write at the next opportunity. This should eliminate a
temporary connection deadlock state on flushing, where the response to the
IHAVE body arrived before we finished writing it. Before, doSomeWrites
would have failed to call issueQUIT because writes were still pending, and
then after the writes complete, there's no code to go back and issue it
until the read timeout expires.
Thanks to Russ Allbery for the patch.
Modified:
branches/2.4/innfeed/connection.c
--------------+
connection.c | 41 +++++++++++++++++++++++++++--------------
1 file changed, 27 insertions(+), 14 deletions(-)
Modified: connection.c
===================================================================
--- connection.c 2008-04-06 15:13:22 UTC (rev 7752)
+++ connection.c 2008-04-09 18:05:59 UTC (rev 7753)
@@ -2329,10 +2329,16 @@
cxnSleep (cxn) ;
}
else
- /* The article has been sent, so start the response timer. */
- initReadBlockedTimeout (cxn) ;
+ {
+ /* Some hosts return a response even before we're done sending, so don't
+ go idle until here. */
+ if (cxn->state == cxnFeedingS && cxn->articleQTotal == 0)
+ cxnIdle (cxn) ;
+ else
+ /* The command set has been sent, so start the response timer. */
+ initReadBlockedTimeout (cxn) ;
+ }
-
freeBufferArray (b) ;
return ;
@@ -2382,8 +2388,8 @@
}
else
{
- /* Some(?) hosts return the 439 response even before we're done
- sending, so don't go idle until here */
+ /* Some hosts return a response even before we're done sending, so don't
+ go idle until here. */
if (cxn->state == cxnFeedingS && cxn->articleQTotal == 0)
cxnIdle (cxn) ;
else
@@ -2716,6 +2722,11 @@
* take a long time. Instead the Connection just tries its next article on
* tape or queue, and if that's no good then it registers this callback so
* that other Connections have a chance of being serviced.
+ *
+ * This function is also put on the callback queue if we called doSomeWrites
+ * but there was already a write pending, mostly so that we don't deadlock the
+ * connection when we got a response to the body of an IHAVE command before we
+ * finished sending the body.
*/
static void cxnWorkProc (EndPoint ep UNUSED, void *data)
{
@@ -2940,7 +2951,7 @@
else
{
remArtHolder (artHolder, &cxn->checkRespHead, &cxn->articleQTotal) ;
- if (cxn->articleQTotal == 0)
+ if (cxn->articleQTotal == 0 && !writeIsPending (cxn->myEp))
cxnIdle (cxn) ;
hostArticleDeferred (cxn->myHost, cxn, artHolder->article) ;
delArtHolder (artHolder) ;
@@ -3001,7 +3012,7 @@
cxn->checksRefused++ ;
remArtHolder (artHolder, &cxn->checkRespHead, &cxn->articleQTotal) ;
- if (cxn->articleQTotal == 0)
+ if (cxn->articleQTotal == 0 && !writeIsPending (cxn->myEp))
cxnIdle (cxn) ;
hostArticleNotWanted (cxn->myHost, cxn, artHolder->article);
delArtHolder (artHolder) ;
@@ -3062,7 +3073,7 @@
cxn->takesSizeOkayed += artSize(artHolder->article);
remArtHolder (artHolder, &cxn->takeRespHead, &cxn->articleQTotal) ;
- if (cxn->articleQTotal == 0)
+ if (cxn->articleQTotal == 0 && !writeIsPending (cxn->myEp))
cxnIdle (cxn) ;
hostArticleAccepted (cxn->myHost, cxn, artHolder->article) ;
delArtHolder (artHolder) ;
@@ -3156,7 +3167,7 @@
{
cxn->checksRefused++ ;
remArtHolder (artHolder, &cxn->checkRespHead, &cxn->articleQTotal) ;
- if (cxn->articleQTotal == 0)
+ if (cxn->articleQTotal == 0 && !writeIsPending (cxn->myEp))
cxnIdle (cxn) ;
hostArticleNotWanted (cxn->myHost, cxn, artHolder->article);
delArtHolder (artHolder) ;
@@ -3172,8 +3183,7 @@
cxn->takesSizeRejected += artSize(artHolder->article);
remArtHolder (artHolder, &cxn->takeRespHead, &cxn->articleQTotal) ;
- /* Some(?) hosts return the 439 response even before we're done
- sending */
+ /* Some hosts return the 439 response even before we're done sending */
if (cxn->articleQTotal == 0 && !writeIsPending(cxn->myEp))
cxnIdle (cxn) ;
hostArticleRejected (cxn->myHost, cxn, artHolder->article) ;
@@ -3237,7 +3247,7 @@
cxn->takesOkayed++ ;
cxn->takesSizeOkayed += artSize(artHolder->article);
- if (cxn->articleQTotal == 0)
+ if (cxn->articleQTotal == 0 && !writeIsPending (cxn->myEp))
cxnIdle (cxn) ;
hostArticleAccepted (cxn->myHost, cxn, artHolder->article) ;
@@ -3917,7 +3927,10 @@
/* If there's a write pending we can't do anything now. */
if ( writeIsPending (cxn->myEp) )
- return ;
+ {
+ addWorkCallback (cxn->myEp,cxnWorkProc,cxn) ;
+ return ;
+ }
else if ( writesNeeded (cxn) ) /* something on a queue. */
{
if (cxn->doesStreaming)
@@ -4157,7 +4170,7 @@
was a big backlog of missing articles *and* we're running in
no-CHECK mode, then the Host would be putting bad articles on the
queue we're taking them off of. */
- if (cxn->missing && cxn->articleQTotal == 0)
+ if (cxn->missing && cxn->articleQTotal == 0 && !writeIsPending (cxn->myEp))
cxnIdle (cxn) ;
for (p = cxn->missing ; p != NULL ; p = q)
{
More information about the inn-committers
mailing list