innfeed: cxnsleep can't write command: Bad address [PATCH]
"Miquel van Smoorenburg"
list-inn-workers at news.cistron.nl
Mon May 2 22:58:28 UTC 2005
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.
Patch below.
diff -ruN t/inn-2.4.2/innfeed/article.c inn-2.4.2/innfeed/article.c
--- t/inn-2.4.2/innfeed/article.c 2004-12-22 05:21:19.000000000 +0100
+++ inn-2.4.2/innfeed/article.c 2005-05-03 00:16:02.000000000 +0200
@@ -44,6 +44,16 @@
extern bool useMMap ;
+struct map_info_s {
+ ARTHANDLE *arthandle;
+ const void *mMapping;
+ size_t size;
+ int refCount;
+ struct map_info_s *next;
+};
+
+typedef struct map_info_s *MapInfo;
+
struct article_s
{
int refCount ; /* the reference count on this article */
@@ -51,11 +61,10 @@
char *msgid ; /* the msgid of the article (INN tells us) */
Buffer contents ; /* the buffer of the actual on disk stuff */
Buffer *nntpBuffers ; /* list of buffers for transmisson */
- const void *mMapping ; /* base of memory mapping, or NULL if none */
+ MapInfo mapInfo; /* arthandle and mMapping */
bool loggedMissing ; /* true if article is missing and we logged */
bool articleOk ; /* true until we know otherwise. */
bool inWireFormat ; /* true if ->contents is \r\n/dot-escaped */
- ARTHANDLE *arthandle ; /* Storage manager article handle */
} ;
struct hash_entry_s {
@@ -93,7 +102,7 @@
its contents buffer if
possible. */
-static void artUnmap (Article art, size_t size) ; /* munmap an mmap()ed
+static void artUnmap (Article art) ; /* munmap an mmap()ed
article */
@@ -156,7 +165,7 @@
static TimeoutId articleStatsId ; /* The timer callback id. */
-
+static MapInfo mapInfo;
/*
* Hash Table data
@@ -209,12 +218,11 @@
newArt->msgid = xstrdup (msgid) ;
newArt->contents = NULL ;
- newArt->mMapping = NULL ;
+ newArt->mapInfo = NULL ;
newArt->refCount = 1 ;
newArt->loggedMissing = false ;
newArt->articleOk = true ;
newArt->inWireFormat = false ;
- newArt->arthandle = NULL;
d_printf (3,"Adding a new article(%p): %s\n", (void *)newArt, msgid) ;
@@ -257,8 +265,8 @@
if (article->contents != NULL)
{
- if (article->mMapping)
- artUnmap(article, bufferSize(article->contents)) ;
+ if (article->mapInfo)
+ artUnmap(article);
else
bytesInUse -= bufferDataSize (article->contents) ;
@@ -464,17 +472,67 @@
return rval ;
}
+ /* arthandle/mMapping needs to be refcounted since a buffer
+ may exist referencing it after delArticle if the remote
+ host sends the return status too soon (diablo, 439). */
+
+static MapInfo getMapInfo(ARTHANDLE *arthandle,
+ const void *mMapping, size_t size)
+{
+ MapInfo m;
+
+ for (m = mapInfo; m; m = m->next) {
+ if (m->arthandle == arthandle &&
+ m->mMapping == mMapping) {
+ m->refCount++;
+ return m;
+ }
+ }
+
+ m = xmalloc(sizeof(struct map_info_s));
+ m->refCount = 1;
+ m->arthandle = arthandle;
+ m->mMapping = mMapping;
+ m->size = size;
+ m->next = mapInfo;
+ mapInfo = m;
+
+ return m;
+}
+
+static void delMapInfo(void *vm)
+{
+ MapInfo i, prev;
+ MapInfo m = (MapInfo)vm;
-static void artUnmap (Article article, size_t size) {
-
- if (article->arthandle)
- SMfreearticle(article->arthandle);
+ if (m == NULL)
+ return;
+
+ if (--(m->refCount) > 0)
+ return;
+
+ if (m->arthandle)
+ SMfreearticle(m->arthandle);
+ else
+ if (munmap(m->mMapping, m->size) < 0)
+ syslog (LOG_NOTICE, "munmap article: %m");
+
+ prev = NULL;
+ for (i = mapInfo; i != m; i = i->next)
+ prev = i;
+
+ if (prev)
+ prev->next = m->next;
else
- if (munmap(article->mMapping, size) < 0)
- syslog (LOG_NOTICE, "munmap %s: %m", article->fname) ;
+ mapInfo = m->next;
+
+ free(m);
+}
+
+static void artUnmap (Article article) {
- article->arthandle = NULL;
- article->mMapping = NULL ;
+ delMapInfo(article->mapInfo);
+ article->mapInfo = NULL;
}
@@ -513,7 +571,9 @@
size_t idx = 0, amtToRead ;
size_t newBufferSize ;
HashEntry h ;
-
+ ARTHANDLE *arthandle = NULL;
+ const void *mMapping;
+
ASSERT (article->contents == NULL) ;
TMRstart(TMR_READART);
@@ -524,9 +584,9 @@
avgCharsPerLine = 75 ; /* roughly number of characters per line */
if (IsToken(article->fname)) {
- opened = ((article->arthandle = SMretrieve(TextToToken(article->fname), RETR_ALL)) != NULL) ? true : false;
+ opened = ((arthandle = SMretrieve(TextToToken(article->fname), RETR_ALL)) != NULL) ? true : false;
if (opened)
- articlesize = article->arthandle->len;
+ articlesize = arthandle->len;
else {
if (SMerrno != SMERR_NOENT && SMerrno != SMERR_UNINIT) {
syslog(LOG_ERR, "Could not retrieve %s: %s",
@@ -540,7 +600,7 @@
struct stat sb ;
opened = ((fd = open (article->fname,O_RDONLY,0)) >= 0) ? true : false;
- article->arthandle = NULL;
+ arthandle = NULL;
if (opened) {
if (fstat (fd, &sb) < 0) {
article->articleOk = false ;
@@ -580,23 +640,27 @@
amtToRead = articlesize ;
newBufferSize = articlesize ;
- if (article->arthandle || useMMap) {
- if (article->arthandle)
- article->mMapping = article->arthandle->data;
+ if (arthandle || useMMap) {
+ if (arthandle)
+ mMapping = arthandle->data;
else
- article->mMapping = mmap(NULL, articlesize, PROT_READ,
+ mMapping = mmap(NULL, articlesize, PROT_READ,
MAP_SHARED, fd, 0);
- if (article->mMapping == MAP_FAILED) {
+ if (mMapping == MAP_FAILED) {
/* dunno, but revert to plain reading */
- article->mMapping = NULL ;
+ mMapping = NULL ;
syswarn ("ME mmap failure %s (%s)",
article->fname,
strerror(errno)) ;
} else {
- article->contents = newBufferByCharP(article->mMapping,
+ article->contents = newBufferByCharP(mMapping,
articlesize,
articlesize);
+ article->mapInfo = getMapInfo(arthandle, mMapping, articlesize);
+ article->mapInfo->refCount++; /* one more for the buffer */
+ bufferSetDeletedCbk(article->contents, delMapInfo, article->mapInfo);
+
buffer = bufferBase (article->contents) ;
if ((p = strchr(buffer, '\n')) == NULL) {
article->articleOk = false;
@@ -648,9 +712,9 @@
byteTotal += articlesize ;
}
- if (article->mMapping && buffer != NULL) {
- memcpy(buffer, article->mMapping, articlesize);
- artUnmap(article, articlesize) ;
+ if (mMapping && buffer != NULL) {
+ memcpy(buffer, mMapping, articlesize);
+ artUnmap(article) ;
amtToRead = 0;
}
@@ -721,7 +785,7 @@
/* If we're not useing storage api, we should close a valid file descriptor */
- if (!article->arthandle && (fd >= 0))
+ if (!arthandle && (fd >= 0))
close (fd) ;
TMRstop(TMR_READART);
@@ -814,6 +878,10 @@
nntpBuffs [0] = newBufferByCharP (bufferBase (contents),
bufferDataSize (contents),
bufferDataSize (contents)) ;
+ if (article->mapInfo) {
+ article->mapInfo->refCount++;
+ bufferSetDeletedCbk(nntpBuffs[0], delMapInfo, article->mapInfo);
+ }
nntpBuffs [1] = NULL ;
}
@@ -845,8 +913,8 @@
ASSERT (bufferRefCount (art->contents) == 1) ;
- if (art->mMapping)
- artUnmap(art, bufferSize(art->contents)) ;
+ if (art->mapInfo)
+ artUnmap(art);
else
bytesInUse -= bufferDataSize (art->contents) ;
diff -ruN t/inn-2.4.2/innfeed/buffer.c inn-2.4.2/innfeed/buffer.c
--- t/inn-2.4.2/innfeed/buffer.c 2004-12-22 05:21:19.000000000 +0100
+++ inn-2.4.2/innfeed/buffer.c 2005-05-03 00:17:01.000000000 +0200
@@ -32,6 +32,8 @@
size_t memSize ; /* the length of mem */
size_t dataSize ; /* amount that has actual data in it. */
bool deletable ;
+ void (*bufferDeletedCbk)(void *);
+ void *bufferDeletedCbkData;
struct buffer_s *next ;
struct buffer_s *prev ;
};
@@ -69,6 +71,8 @@
nb->memSize = size ;
nb->dataSize = 0 ;
nb->deletable = true ;
+ nb->bufferDeletedCbk = NULL;
+ nb->bufferDeletedCbkData = NULL;
bufferByteCount += size + 1 ;
bufferCount++ ;
@@ -103,6 +107,8 @@
nb->memSize = size ;
nb->dataSize = dataSize ;
nb->deletable = false ;
+ nb->bufferDeletedCbk = NULL;
+ nb->bufferDeletedCbkData = NULL;
nb->next = gBufferList ;
nb->prev = NULL;
@@ -136,6 +142,12 @@
buff->mem = NULL ;
}
+ if (buff->bufferDeletedCbk) {
+ (buff->bufferDeletedCbk)(buff->bufferDeletedCbkData);
+ buff->bufferDeletedCbk = NULL;
+ buff->bufferDeletedCbkData = NULL;
+ }
+
if (buff->next != NULL)
buff->next->prev = buff->prev ;
if (buff->prev != NULL)
@@ -250,6 +262,15 @@
ASSERT (buff->dataSize <= buff->memSize) ;
}
+void bufferSetDeletedCbk (Buffer buff, void (*cbk)(void *), void *data)
+{
+ ASSERT(buff->bufferDeletedCbk == NULL &&
+ buff->bufferDeletedCbk != cbk);
+ ASSERT(buff->bufferDeletedCbkData == NULL &&
+ buff->bufferDeletedCbkData != data);
+ buff->bufferDeletedCbk = cbk;
+ buff->bufferDeletedCbkData = data;
+}
void freeBufferArray (Buffer *buffs)
{
diff -ruN t/inn-2.4.2/innfeed/buffer.h inn-2.4.2/innfeed/buffer.h
--- t/inn-2.4.2/innfeed/buffer.h 2004-12-22 05:21:19.000000000 +0100
+++ inn-2.4.2/innfeed/buffer.h 2005-05-01 22:39:41.000000000 +0200
@@ -73,6 +73,9 @@
/* set the size of the data actually in the buffer */
void bufferSetDataSize (Buffer buff, size_t size) ;
+/* callback to be called when buffer gets deleted */
+void bufferSetDeletedCbk (Buffer buff, void (*cbk)(void *), void *data);
+
/* walk down the BUFFS releasing each buffer and then freeing BUFFS itself */
void freeBufferArray (Buffer *buffs) ;
--
The From: and Reply-To: addresses are internal news2mail gateway addresses.
Reply to the list or to "Miquel van Smoorenburg" <miquels at cistron.nl>
More information about the inn-workers
mailing list