[Box Backup-dev] Bug: Compare stopping on locked files

[Pascal Lalonde]

This is a multi-part message in MIME format.

--Boundary_(ID_N5Hbf33m9soB/Sg9AIxZHA)
Content-type: text/plain; charset=ISO-8859-1; format=flowed
Content-transfer-encoding: 7BIT

On win32, when using bbackupquery, a compare command fails for a given
location when it encounters a locked file, which is so common in
Windows, especially on multiuser systems.

I never get to see a complete compare.

This is the output from the command-line:

---------------
Box Backup Query Tool vtrunk_625, (c) Ben Summers and contributors 2003-2006
Using configuration file bbackupd.conf
Connecting to store...
Handshake with store...
Login to store...
Login complete.

Type "help" for a list of commands.

WARNING: Quick compare used -- file attributes are not checked.
Unable to send message to Event Log: error 6:
Failed to open file C:\miranda\\blah.dat: error 32
ERROR: (1/2) during file fetch and comparison for '/miranda/blah.dat'
ERROR: (7/42) during file fetch and comparison for '/miranda/miranda32.exe'
ERROR: (7/42) during file fetch and comparison for 
'/miranda/mirandaboot.ini'

[ 0 (of 0) differences probably due to file modifications after the last 
upload
]
Differences: 0 (0 dirs excluded, 0 files excluded)
Logging off...
Exception: Connection Protocol_ObjTooBig (7/42)
---------------


Error 32 means a sharing violation:
http://msdn.microsoft.com/library/en-us/debug/base/system_error_codes__0-499_.asp?frame=true

It should continue even when encountering such a file.

After digging in the code, I noticed that this bug should affect the 
*nix version as well, but since mandatory locks seldom occur, this bug 
has never bitten me on Linux and OpenBSD.

The problem occurs in BackupQueries.cpp, at line 1425 (trunk, rev. 626). 
Since the file is locked, the FileStream() constructor in 
CompareFileContentsAgainstBlockIndex() throws an exception, which goes 
all to the catch() block where an error is printed, and then the loop 
proceeds to the next file. The problem is, before that throw(), the 
download for our locked file had already been "scheduled". Thus, when 
reading the stream, I guess what we get is the file from the previous 
iteration since mrConnection is not flushed on error.

Here is a patch which fixes this issue. I'm not sure if it is the right 
solution, but it should at least outline the source of the problem.

Pascal

--Boundary_(ID_N5Hbf33m9soB/Sg9AIxZHA)
Content-type: text/x-patch; CHARSET=US-ASCII; name=compare_bug.diff
Content-transfer-encoding: 7BIT
Content-disposition: inline; filename=compare_bug.diff

Index: lib/backupclient/BackupStoreFile.cpp
===================================================================
--- lib/backupclient/BackupStoreFile.cpp	(revision 626)
+++ lib/backupclient/BackupStoreFile.cpp	(working copy)
@@ -1225,7 +1225,51 @@
 }
 
 
+// --------------------------------------------------------------------------
+//
+// Function
+//		Name:    BackupStoreFile::FlushStream(IOStream &, int)
+//		Purpose: Flushes the stream. Returns true on success.
+//		Created: 20/6/06
+//
+// --------------------------------------------------------------------------
+bool BackupStoreFile::FlushStream(IOStream &rBlockIndex, int Timeout)
+{
+	// Read header
+	file_BlockIndexHeader hdr;
+	if(!rBlockIndex.ReadFullBuffer(&hdr, sizeof(hdr), 0 /* not interested in bytes read if this fails */, Timeout))
+	{
+		// Couldn't read header
+		THROW_EXCEPTION(BackupStoreException, CouldntReadEntireStructureFromStream)
+	}
 
+	// Check magic
+	if(hdr.mMagicValue != (int32_t)htonl(OBJECTMAGIC_FILE_BLOCKS_MAGIC_VALUE_V1)
+#ifndef BOX_DISABLE_BACKWARDS_COMPATIBILITY_BACKUPSTOREFILE
+		&& hdr.mMagicValue != (int32_t)htonl(OBJECTMAGIC_FILE_BLOCKS_MAGIC_VALUE_V0)
+#endif
+		)
+	{
+		THROW_EXCEPTION(BackupStoreException, BadBackupStoreFile)
+	}
+
+	// Get basic information
+	int64_t numBlocks = box_ntoh64(hdr.mNumBlocks);
+	
+	for(int64_t b = 0; b < numBlocks; ++b)
+	{
+		// Read an entry from the stream
+		file_BlockIndexEntry entry;
+		if(!rBlockIndex.ReadFullBuffer(&entry, sizeof(entry), 0 /* not interested in bytes read if this fails */, Timeout))
+		{
+			// Couldn't read entry
+			THROW_EXCEPTION(BackupStoreException, CouldntReadEntireStructureFromStream)
+		}	
+	}
+	
+	return true;
+}
+
 // --------------------------------------------------------------------------
 //
 // Function
Index: lib/backupclient/BackupStoreFile.h
===================================================================
--- lib/backupclient/BackupStoreFile.h	(revision 626)
+++ lib/backupclient/BackupStoreFile.h	(working copy)
@@ -133,6 +133,7 @@
 	static void DecodeFile(IOStream &rEncodedFile, const char *DecodedFilename, int Timeout, const BackupClientFileAttributes *pAlterativeAttr = 0);
 	static std::auto_ptr<BackupStoreFile::DecodedStream> DecodeFileStream(IOStream &rEncodedFile, int Timeout, const BackupClientFileAttributes *pAlterativeAttr = 0);
 	static bool CompareFileContentsAgainstBlockIndex(const char *Filename, IOStream &rBlockIndex, int Timeout);
+	static bool FlushStream(IOStream &rBlockIndex, int Timeout);
 	static std::auto_ptr<IOStream> CombineFileIndices(IOStream &rDiff, IOStream &rFrom, bool DiffIsIndexOnly = false, bool FromIsIndexOnly = false);
 
 	// Stream manipulation
Index: bin/bbackupquery/BackupQueries.cpp
===================================================================
--- bin/bbackupquery/BackupQueries.cpp	(revision 626)
+++ bin/bbackupquery/BackupQueries.cpp	(working copy)
@@ -1422,7 +1422,15 @@
 						std::auto_ptr<IOStream> blockIndexStream(mrConnection.ReceiveStream());
 						
 						// Compare
-						equal = BackupStoreFile::CompareFileContentsAgainstBlockIndex(localPath.c_str(), *blockIndexStream, mrConnection.GetTimeout());
+						try
+						{
+							equal = BackupStoreFile::CompareFileContentsAgainstBlockIndex(localPath.c_str(), *blockIndexStream, mrConnection.GetTimeout());
+						}
+						catch(...)
+						{
+							BackupStoreFile::FlushStream(*blockIndexStream, mrConnection.GetTimeout());
+							throw;
+						}
 					}
 					else
 					{

--Boundary_(ID_N5Hbf33m9soB/Sg9AIxZHA)--