Validate operations on FileHandles (#1113)

* Validate operations on FileHandles

This commit allows State to track whether a file handle is valid for read etc
operations. Our GC mechanism keeps track of the open-handles but assumes the
correctness of callers, meaning if multiple calls to `release` are made at the
filesystem level a node may be GC'ed before its time.

The State now keeps a record of which FileHandles exist and whether they remain
valid, meaning the GC mechanism cannot be misled.

REF SMPTNG-532

Signed-off-by: Brian L. Troutwine <[email protected]>

* accidentally dropped a counter

Signed-off-by: Brian L. Troutwine <[email protected]>

* scoot advance time back

Signed-off-by: Brian L. Troutwine <[email protected]>

* indentation

Signed-off-by: Brian L. Troutwine <[email protected]>

---------

Signed-off-by: Brian L. Troutwine <[email protected]>

Author: blt

lading/src/generator/file_gen/logrotate_fs.rs

@@ -370,14 +370,14 @@ impl Filesystem for LogrotateFS {
 
         counter!("fs_release").increment(1);
 
-        // Remove the FileHandle from the mapping
+        // Remove `fh->FileHandle` from the set of open_files.
         let file_handle = {
             let mut open_files = self.open_files.lock().expect("lock poisoned");
             open_files.remove(&fh)
         };
 
         if let Some(file_handle) = file_handle {
-            // Close the file in the state
+            // Close the file in the model
             state.close_file(tick, file_handle);
             reply.ok();
         } else {

lading/src/generator/file_gen/logrotate_fs/model.rs

@@ -298,6 +298,7 @@ pub(crate) struct State {
     next_file_handle: u64,
     inode_scratch: Vec<Inode>,
     load_profile: LoadProfile,
+    valid_file_handles: FxHashMap<u64, Inode>, // Track valid FileHandle IDs -> Inode
 }
 
 impl std::fmt::Debug for State {
@@ -387,6 +388,7 @@ impl State {
             next_file_handle: 0,
             inode_scratch: Vec::with_capacity(concurrent_logs as usize),
             load_profile,
+            valid_file_handles: FxHashMap::default(),
         };
 
         if concurrent_logs == 0 {
@@ -501,6 +503,7 @@ impl State {
             file.open(now);
             let id = self.next_file_handle;
             self.next_file_handle = self.next_file_handle.wrapping_add(1);
+            self.valid_file_handles.insert(id, inode);
             Some(FileHandle { id, inode })
         } else {
             None
@@ -519,6 +522,7 @@ impl State {
 
         if let Some(Node::File { file, .. }) = self.nodes.get_mut(&handle.inode) {
             file.close(now);
+            self.valid_file_handles.remove(&handle.id);
         } else {
             panic!("Invalid file handle");
         }
@@ -816,6 +820,17 @@ impl State {
     ) -> Option<Bytes> {
         self.advance_time(now);
 
+        // Check if the FileHandle is still valid and maps to the correct inode
+        if let Some(&inode) = self.valid_file_handles.get(&file_handle.id) {
+            // Doesn't match the node.
+            if inode != file_handle.inode {
+                return None;
+            }
+        } else {
+            // No longer valid.
+            return None;
+        }
+
         let inode = file_handle.inode;
         match self.nodes.get_mut(&inode) {
             Some(Node::File { ref mut file }) => {
@@ -1240,6 +1255,45 @@ mod test {
                 );
             }
         }
+
+        // Property 9: Unlinked files remain readable so long as there is a
+        // valid file handle.
+        //
+        // Files that are unlinked and read-only should remain in the state's
+        // nodes as long as they have open handles. They should be removed only
+        // when open_handles == 0. Reads should still be possible via valid open
+        // file handles.
+        for (&inode, node) in &state.nodes {
+            if let Node::File { file } = node {
+                if file.unlinked && file.read_only {
+                    if file.open_handles > 0 {
+                        // Should remain in state.nodes
+                        assert!(state.nodes.contains_key(&inode),
+                                "Unlinked, read-only file with open handles should remain in state.nodes");
+
+                        // There should be valid file handles pointing to this inode
+                        let valid_handles: Vec<_> = state
+                            .valid_file_handles
+                            .iter()
+                            .filter_map(|(&handle_id, &fh_inode)| {
+                                if fh_inode == inode {
+                                    Some(handle_id)
+                                } else {
+                                    None
+                                }
+                            })
+                            .collect();
+
+                        assert!(!valid_handles.is_empty(),
+                                "Unlinked, read-only file with open handles should have valid file handles");
+                    } else {
+                        // Should be removed from state.nodes after GC
+                        assert!(!state.nodes.contains_key(&inode),
+                                "Unlinked, read-only file with zero open handles should be removed from state.nodes");
+                    }
+                }
+            }
+        }
     }
 
     fn compute_expected_bytes_written(