dovecot: Better race condition fix for mmaped log files
dovecot at dovecot.org
dovecot at dovecot.org
Thu Jun 21 03:42:15 EEST 2007
details: http://hg.dovecot.org/dovecot/rev/e5da155f44c3
changeset: 5796:e5da155f44c3
user: Timo Sirainen <tss at iki.fi>
date: Thu Jun 21 03:41:56 2007 +0300
description:
Better race condition fix for mmaped log files
diffstat:
1 file changed, 25 insertions(+), 23 deletions(-)
src/lib-index/mail-transaction-log-file.c | 48 +++++++++++++++--------------
diffs (65 lines):
diff -r 0c0a829a9a63 -r e5da155f44c3 src/lib-index/mail-transaction-log-file.c
--- a/src/lib-index/mail-transaction-log-file.c Thu Jun 21 03:32:23 2007 +0300
+++ b/src/lib-index/mail-transaction-log-file.c Thu Jun 21 03:41:56 2007 +0300
@@ -727,36 +727,38 @@ mail_transaction_log_file_sync(struct ma
trans_size = 0;
}
+ if (file->mmap_base != NULL && !file->locked) {
+ /* Now that all the mmaped pages have page faulted, check if
+ the file had changed while doing that. Only after the last
+ page has faulted, the size returned by fstat() can be
+ trusted. Otherwise it might point to a page boundary while
+ the next page is still being written.
+
+ Without this check we might see partial transactions,
+ sometimes causing "Extension record updated without intro
+ prefix" errors. */
+ if (fstat(file->fd, &st) < 0) {
+ mail_index_file_set_syscall_error(file->log->index,
+ file->filepath,
+ "fstat()");
+ return -1;
+ }
+ if ((uoff_t)st.st_size != file->last_size) {
+ file->last_size = st.st_size;
+ return 0;
+ }
+ }
+
avail = file->sync_offset - file->buffer_offset;
if (avail != size) {
/* There's more data than we could sync at the moment. If the
last record's size wasn't valid, we can't know if it will
be updated unless we've locked the log.
- If the last record size was valid but there isn't enough
- data for it, it might or might not be an error:
-
- 1. If we're locked, it's definitely an error.
- 2. If not and we're using pread()s, it's also an error.
- 3. If not and we're using mmap()s, we can't be sure yet.
- The data might have changed, so we'll need to fstat()
- to check if the file size had changed. If it did, return 0
- so the caller will re-mmap() and resync the file. */
+ If the record size was valid, this is an error because the
+ pread()s or the above fstat() check for mmaps should have
+ guaranteed that this doesn't happen. */
if (file->locked || trans_size != 0) {
- if (file->mmap_base != NULL && !file->locked) {
- i_assert(trans_size != 0);
-
- if (fstat(file->fd, &st) < 0) {
- mail_index_file_set_syscall_error(
- file->log->index,
- file->filepath, "fstat()");
- return -1;
- }
- if ((uoff_t)st.st_size != file->last_size) {
- file->last_size = st.st_size;
- return 0;
- }
- }
if (trans_size != 0) {
mail_transaction_log_file_set_corrupted(file,
"hdr.size too large (%u)", trans_size);
More information about the dovecot-cvs
mailing list