From d172ee84c3cf3fdd8c10a3d672de1d28d1da7e9e Mon Sep 17 00:00:00 2001
From: Ingo Weinhold <ingo_weinhold@gmx.de>
Date: Sat, 2 Aug 2008 17:41:03 +0000
Subject: [PATCH] * Fixed recently introduced write lock leak, when something
 failed   before file_cache_write() or nothing had to be written. * Fixed race
 condition. While neither transaction nor read lock are   held, the file size
 can change. * Add the inode to the transaction whenever possible, i.e. on
 error   before file_cache_write() and after it as well. This should prevent  
 readers from seeing inconsistent blocks when the transaction has to be  
 rolled back.

git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@26736 a95241bf-73f2-0310-859d-f6bbb57e9c96
---
 src/add-ons/kernel/file_systems/bfs/Inode.cpp | 34 ++++++++++++-------
 1 file changed, 21 insertions(+), 13 deletions(-)

diff --git a/src/add-ons/kernel/file_systems/bfs/Inode.cpp b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
index 02719f0e7c..3f18f831c4 100644
--- a/src/add-ons/kernel/file_systems/bfs/Inode.cpp
+++ b/src/add-ons/kernel/file_systems/bfs/Inode.cpp
@@ -1412,33 +1412,34 @@ Inode::WriteAt(Transaction& transaction, off_t pos, const uint8* buffer,
 	// TODO: support INODE_LOGGED!
 
 	size_t length = *_length;
-	bool changeSize = false;
+	bool changeSize = pos + length > Size();
 
 	// set/check boundaries for pos/length
 	if (pos < 0)
 		return B_BAD_VALUE;
 
-	if (pos + length > Size())
-		changeSize = true;
-
 	locker.Unlock();
 
 	// the transaction doesn't have to be started already
 	if (changeSize && !transaction.IsStarted())
 		transaction.Start(fVolume, BlockNumber());
 
-	// TODO: we actually need to call WriteLockInTransaction() here, but we
-	// cannot do this with the current locking model (ie. file cache functions
-	// are not to be called with the inode lock held).
-	// But this cannot work anyway, since we hold the lock when calling
-	// file_cache_set_size(), too... (possible deadlock)
-	rw_lock_write_lock(&fLock);
+	WriteLocker writeLocker(fLock);
+
+	// Work around possible race condition: Someone might have shrunken the file
+	// while we had no lock.
+	if (!transaction.IsStarted() && pos + length > Size()) {
+		writeLocker.Unlock();
+		transaction.Start(fVolume, BlockNumber());
+		writeLocker.Lock();
+	}
 
 	if (pos + length > Size()) {
 		// let's grow the data stream to the size needed
 		status_t status = SetFileSize(transaction, pos + length);
 		if (status < B_OK) {
 			*_length = 0;
+			WriteLockInTransaction(transaction);
 			RETURN_ERROR(status);
 		}
 		// TODO: In theory we would need to update the file size
@@ -1449,18 +1450,25 @@ Inode::WriteAt(Transaction& transaction, off_t pos, const uint8* buffer,
 		// go into this transaction (we cannot wait until the file
 		// is closed)
 		status = WriteBack(transaction);
-		if (status < B_OK)
+		if (status < B_OK) {
+			WriteLockInTransaction(transaction);
 			return status;
+		}
 	}
 
+	writeLocker.Unlock();
+
 	// If we don't want to write anything, we can now return (we may
 	// just have changed the file size using the position parameter)
 	if (length == 0)
 		return B_OK;
 
-	rw_lock_write_unlock(&fLock);
+	status_t status = file_cache_write(FileCache(), NULL, pos, buffer, _length);
 
-	return file_cache_write(FileCache(), NULL, pos, buffer, _length);
+	if (transaction.IsStarted())
+		WriteLockInTransaction(transaction);
+
+	return status;
 }