Repository: paritytech/parity-db (language: rust)

Issue:
Slice indexation error after BTree header write failure
First bug uncovered thanks to #113.

When a partial write happens after opening the database with a single BTree the iteration fails on the following db opening with `'range end index 8 out of range for slice of length 0'` inside of `Entry::read_slice` nested in `BTreeTable::btree_header`.

Reproduction test (requires #113 and the enabeling of the `instrumentation` feature):
```rust
	#[test]
	fn test_from_fuzzer() {
		crate::set_number_of_allowed_io_operations(12);
		let tmp = tempdir().unwrap();
		let mut options = Options::with_columns(tmp.path(), 1);
		options.columns[0].btree_index = true;
		assert!(Db::open_or_create(&options).is_err()); //It's going to fail

		let db = Db::open_or_create(&options).unwrap();
		db.iter(0).unwrap(); //Read is panicking here
	}
```


Discussion / hints:
The wal should prevent that, but I remember skipping the wal for some first time initialization of the header (only on the very first db opening), will check that next week.
> I remember skipping the wal for some first time initialization of the header

Thank you ! It looks a lot like it.
I had a deeper look it it. Fixing this cleanly by adding the header initialization to the log seems to require a significant refactoring to avoid piling up hacks.

@arkpar Is this something important to fix? The column initialization is likely to only happen at the database creation, hence at a step where there is no data to corrupt.
Not that important really. 

Performing initialization within an internal transaction that involves log would be one way to fix it. Another way is not to initialize at all, until there's a first transaction that inserts anything into the column. Just treat the missing file/header as an empty header.

Another option: handle write errors on initialization and cleanup (delete) the file properly if anything goes wrong, before propagating the error up.
> Another option: handle write errors on initialization and cleanup (delete) the file properly if anything goes wrong, before propagating the error up.

I would maybe go in this direction (using log is a lot of work for something small and treating missing file header as empty header was a bit awkward iirc (like I did not want to have an api with bound check on file read, but maybe checking file size could have done the job and I just did not think of doing that at the time).
> Another option: handle write errors on initialization and cleanup (delete) the file properly if anything goes wrong, before propagating the error up.

It sounds indeed a good way to go. Thank you!

Fix patch:
diff --git a/src/file.rs b/src/file.rs
index 07755858..f23baa6a 100644
--- a/src/file.rs
+++ b/src/file.rs
@@ -211,4 +211,13 @@ impl TableFile {
 		}
 		Ok(())
 	}
+
+	pub fn remove(&self) -> Result<()> {
+		let mut file = self.file.write();
+		if let Some(file) = file.take() {
+			drop(file);
+			try_io!(std::fs::remove_file(&self.path));
+		}
+		Ok(())
+	}
 }
diff --git a/src/table.rs b/src/table.rs
index b089c8be..dd8d730b 100644
--- a/src/table.rs
+++ b/src/table.rs
@@ -970,6 +970,15 @@ impl ValueTable {
 	}
 
 	pub fn init_with_entry(&self, entry: &[u8]) -> Result<()> {
+		if let Err(e) = self.do_init_with_entry(entry) {
+			log::error!(target: "parity-db", "Failure to initialize file {}", self.file.path.display());
+			let _ = self.file.remove(); // We ignore error here
+			return Err(e)
+		}
+		Ok(())
+	}
+
+	fn do_init_with_entry(&self, entry: &[u8]) -> Result<()> {
 		self.file.grow(self.entry_size)?;
 
 		let empty_overlays = parking_lot::RwLock::new(Default::default());


Test patch: