Re: innfeed: cxnsleep can't write command: Bad address [PATCH]
Russ Allbery
> 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 (r...@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.
Miquel van Smoorenburg
Well the buffer really is just a pointer to mmap'ed memory,
which hostArticleRejected is going to unmap. I don't understand
what you mean with "dig the article out of the NNTP buffers".
>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?
That was how I originally tried to fix it. But I didn't like
Articles holding references to Buffers that again hold references
to Articles. delArticle will call delBuffer which will, ehrm,
call delArticle again .. besides how do you get a reference to
the article from within buffer.c.
So I took a different approach.
But, if you keep the bufferSetDeletedCbk infrastructure in place that
I added, I guess you could do it without MapInfo. The callback
would just do a (if article->refCount > 0) delArticle(article),
right ?
Something like
static void delArticleFromBuffer(void *a)
{
Article article = a;
if article->refCount > 0)
delArticle(article);
}
static bool fillContents (Article article)
{
....
article->contents = newBufferByCharP(mMapping,
articlesize,
articlesize);
article->refCount++; /* one more for the buffer */
bufferSetDeletedCbk(article->contents, delArticleFromBuffer, article);
What do you think ?
Mike.
--
The From: and Reply-To: addresses are internal news2mail gateway addresses.
Reply to the list or to "Miquel van Smoorenburg" <miq...@cistron.nl>
Russ Allbery
> Well the buffer really is just a pointer to mmap'ed memory, which
> hostArticleRejected is going to unmap. I don't understand what you mean
> with "dig the article out of the NNTP buffers".
I was thinking that if we know that the peer doesn't want the article, we
should stop sending it, but the more I think about it, the more I realize
that's a stupid idea since then we're sending a corrupted message and if
for some reason the peer actually keeps it, we really screwed up. Not to
mention that doing that is going to be rather hard, given innfeed's
architecture.
> That was how I originally tried to fix it. But I didn't like Articles
> holding references to Buffers that again hold references to
> Articles. delArticle will call delBuffer which will, ehrm, call
> delArticle again .. besides how do you get a reference to the article
> from within buffer.c.
Oh, I see. Because the network sending routines don't have any idea
they're sending articles, just that they're sending buffers, and teaching
them about the articles is pointless complexity.
You're right, your original solution is best, even if we're doing even
more reference counting.