innfeed: cxnsleep can't write command: Bad address [PATCH]

[Russ Allbery]

"Miquel van Smoorenburg" <list-inn-workers at news.cistron.nl> writes:

> I've seen this message regularly in my logs:

> May  2 01:40:47 quantum innfeed[10062]: transit.news.xs4all.nl:6 cxnsleep can't write command: Bad address
> May  2 01:40:47 quantum innfeed[10062]: transit.news.xs4all.nl:6 final seconds 26 offered 249 accepted 59 refused 181 rejected 6 accsize 21955221 rejsize 2292445
> .. but hadn't payed much attention to it. Now that I am using
> innd/innfeed to feed much more diablo boxes a full feed, I'm
> seeing it more and more.

> It turns out that Diablo can return an "article rejected" status
> before the entire article has been transfered. But by then,
> hostArticleRejected has called delArticle and SMfreearticle()
> or munmap() has been called, and the nntpBuffers point to
> data no longer there. Which means that writev() will EFAULT.

> The correct solution is to put arthandle/mMapping in a seperate
> struct and refcount it, only releasing it when the last buffer
> that references it is deleted.

I'm not at all sure that I like this.  It adds another layer of reference
counting to the layer that we already have, and as near as I can tell, is
doing so because the existing reference counting doesn't work properly.
Shouldn't we instead fix the reference counting on articles so that
delArticle won't free the allocated data while it's still referenced
elsewhere?

It seems like the truly correct thing to do here is to teach
hostArticleRejected how to dig the article out of the NNTP buffers for
that host before deleting it.  Failing that, the NNTP buffers should also
hold a reference to the article that they drop when the article has been
written out completely.

Do you agree?  Am I missing something?

-- 
Russ Allbery (rra at stanford.edu)             <http://www.eyrie.org/~eagle/>

    Please send questions to the list rather than mailing me directly.
     <http://www.eyrie.org/~eagle/faqs/questions.html> explains why.