Merge branch 'v5-stable' into v5-devel

Conflicts:
	ChangeLog
	configure.ac
	doc/manual.html

2 files changed, 29 insertions, 11 deletions

diff --git a/ChangeLog b/ChangeLog
index f4084f35..0c66442f 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -7,6 +7,7 @@ Version 5.7.3  [V5-DEVEL] (rgerhards), 2010-12-??
   namely those that have 32bit atomic operations but not 64 bit ones
 - bugfix: local hostname was pulled too-early, so that some config 
   directives (namely FQDN settings) did not have any effect
+- bugfix: imfile did duplicate messages under some circumstances
 ---------------------------------------------------------------------------
 Version 5.7.2  [V5-DEVEL] (rgerhards), 2010-11-26
 - bugfix(important): problem in TLS handling could cause rsyslog to loop
@@ -53,7 +54,7 @@ Version 5.7.0  [V5-DEVEL] (rgerhards), 2010-09-16
     thanks to Lennart Poettering for this patch
   * sd-systemd API added as part of rsyslog runtime library
 ---------------------------------------------------------------------------
-Version 5.6.3  [V5-STABLE] (rgerhards), 2010-12-??
+Version 5.6.3  [V5-STABLE] (rgerhards), 2011-01-26
 - bugfix: action processor released mememory too early, resulting in
   potential issue in retry cases (but very unlikely due to another
   bug, which I also fixed -- only after the fix this problem here
@@ -66,6 +67,7 @@ Version 5.6.3  [V5-STABLE] (rgerhards), 2010-12-??
   it!)
 - bugfix: batches which had actions in error were not properly retried in
   all cases
+- bugfix: imfile did duplicate messages under some circumstances
 ---------------------------------------------------------------------------
 Version 5.6.2  [V5-STABLE] (rgerhards), 2010-11-30
 - bugfix: compile failed on systems without epoll_create1()
@@ -642,6 +644,21 @@ Version 4.7.0  [v4-devel] (rgerhards), 2010-04-14
 - imported changes from 4.5.6 and below
 ---------------------------------------------------------------------------
 Version 4.6.6  [v4-stable] (rgerhards), 2010-11-??
+- bugfix: imfile potentially duplicates lines
+  This can happen when 0 bytes are read from the input file, and some
+  writer appends data to the file BEFORE we check if a rollover happens.
+  The check for rollover uses the inode and size as a criterion. So far,
+  we checked for equality of sizes, which is not given in this scenario,
+  but that does not indicate a rollover. From the source code comments:
+     Note that when we check the size, we MUST NOT check for equality.
+     The reason is that the file may have been written right after we
+     did try to read (so the file size has increased). That is NOT in
+     indicator of a rollover (this is an actual bug scenario we 
+     experienced). So we need to check if the new size is smaller than
+     what we already have seen!
+  Also, under some circumstances an invalid truncation was detected. This
+  code has now been removed, a file change (and thus resent) is only
+  detected if the inode number changes.
 - bugfix: a couple of problems that imfile had on some platforms, namely
   Ubuntu (not their fault, but occured there)
 - bugfix: imfile utilizes 32 bit to track offset. Most importantly,
diff --git a/runtime/stream.c b/runtime/stream.c
index 16d41a2e..5f4249a8 100644
--- a/runtime/stream.c
+++ b/runtime/stream.c
@@ -401,6 +401,12 @@ finalize_it:
  * If we are monitoring a file, someone may have rotated it. In this case, we
  * also need to close it and reopen it under the same name.
  * rgerhards, 2008-02-13
+ * The previous code also did a check for file truncation, in which case the
+ * file was considered rewritten. However, this potential border case turned
+ * out to be a big trouble spot on busy systems. It caused massive message
+ * duplication (I guess stat() can return a too-low number under some
+ * circumstances). So starting as of now, we only check the inode number and
+ * a file change is detected only if the inode changes. -- rgerhards, 2011-01-10
  */
 static rsRetVal
 strmHandleEOFMonitor(strm_t *pThis)
@@ -410,23 +416,18 @@ strmHandleEOFMonitor(strm_t *pThis)
 	struct stat statName;
 
 	ISOBJ_TYPE_assert(pThis, strm);
-	/* find inodes of both current descriptor as well as file now in file
-	 * system. If they are different, the file has been rotated (or
-	 * otherwise rewritten). We also check the size, because the inode
-	 * does not change if the file is truncated (this, BTW, is also a case
-	 * where we actually loose log lines, because we can not do anything
-	 * against truncation...). We do NOT rely on the time of last
-	 * modificaton because that may not be available under all
-	 * circumstances. -- rgerhards, 2008-02-13
-	 */
 	if(fstat(pThis->fd, &statOpen) == -1)
 		ABORT_FINALIZE(RS_RET_IO_ERROR);
 	if(stat((char*) pThis->pszCurrFName, &statName) == -1)
 		ABORT_FINALIZE(RS_RET_IO_ERROR);
-	if(statOpen.st_ino == statName.st_ino && pThis->iCurrOffs == statName.st_size) {
+	DBGPRINTF("stream checking for file change on '%s', inode %u/%u",
+	  pThis->pszCurrFName, (unsigned) statOpen.st_ino,
+	  (unsigned) statName.st_ino);
+	if(statOpen.st_ino == statName.st_ino) {
 		ABORT_FINALIZE(RS_RET_EOF);
 	} else {
 		/* we had a file change! */
+		DBGPRINTF("we had a file change on '%s'\n", pThis->pszCurrFName);
 		CHKiRet(strmCloseFile(pThis));
 		CHKiRet(strmOpenFile(pThis));
 	}