File::close() becomes FileSystem::close(std::shared_ptr<File> file)

examples/jbod.cpp

@@ -31,7 +31,7 @@ class FileWriterActor {
         XBT_INFO("Writing %llu to it...", num_bytes_);
         file->write(num_bytes_);
         XBT_INFO("Closing file...");
-        file->close();
+        fs_->close(file);
         XBT_INFO("The file size it: %llu", fs_->file_size(file_path_));
     }
 };

examples/open_seek_write.cpp

@@ -42,7 +42,7 @@ class FileWriterActor {
         XBT_INFO("Writing %llu to it at offset %llu...", num_bytes_, offset_);
         file->write(num_bytes_);
         XBT_INFO("Closing file...");
-        file->close();
+        fs_->close(file);
         XBT_INFO("The file size it: %llu", fs_->file_size(file_path_));
     }
 };

include/fsmod/File.hpp

@@ -58,8 +58,6 @@ namespace simgrid::module::fs {
         /** Retrieves the current file position */
         [[nodiscard]] sg_size_t tell() const { return current_position_; }
 
-        void close();
-
         std::unique_ptr<FileStat> stat() const;
     };
 

include/fsmod/FileSystem.hpp

@@ -44,6 +44,7 @@ namespace simgrid::module::fs {
         [[nodiscard]] sg_size_t file_size(const std::string& full_path) const;
 
         std::shared_ptr<File> open(const std::string& full_path);
+        void close(std::shared_ptr<File> f);
 
         [[nodiscard]] std::shared_ptr<Partition> partition_by_name(const std::string& name) const;
         [[nodiscard]] std::shared_ptr<Partition> partition_by_name_or_null(const std::string& name) const;

src/File.cpp

@@ -193,14 +193,6 @@ namespace simgrid::module::fs {
         }
     }
 
-    /**
-     * @brief Closes the file. After closing, using the file has undefined
-     * behavior.
-     */
-    void File::close() {
-        metadata_->decrease_file_refcount();
-    }
-
     /**
      * @brief Obtain information about the file as a
      * @return A (

src/FileSystem.cpp

@@ -166,6 +166,16 @@ namespace simgrid::module::fs {
         return file;
     }
 
+   /**
+     * @brief Closes the file. After closing, using the file has undefined
+     * behavior.
+     * @param file a shared pointer on a File
+     */
+    void FileSystem::close(std::shared_ptr<File> file) {
+        this->num_open_files_--;
+        file->metadata_->decrease_file_refcount();
+    }
+
     /**
      * @brief
      * @param full_path: an absolute file path

test/caching_test.cpp

@@ -153,7 +153,7 @@ TEST_F(CachingTest, LRUBasics)  {
             std::shared_ptr<sgfs::File> file;
             ASSERT_NO_THROW(file = fs_->open("/dev/lru/20mb.txt"));
             ASSERT_NO_THROW(file->read(10));
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
             ASSERT_NO_THROW(this->fs_->create_file("/dev/lru/30mb.txt", "30MB"));
             XBT_INFO("Check that files are as they should be");
             ASSERT_TRUE(this->fs_->file_exists("/dev/lru/20mb.txt"));
@@ -201,7 +201,7 @@ TEST_F(CachingTest, LRUExtensive)  {
                     auto file_name = action.at(1);
                     auto file = this->fs_->open("/dev/lru/" + file_name);
                     file->read(1);
-                    file->close();
+                    fs_->close(file);
                 }
                 // Check state
                 for (auto const &f : expected_files_there) {

test/file_system_test.cpp

@@ -131,7 +131,7 @@ TEST_F(FileSystemTest, Directories)  {
             std::shared_ptr<sgfs::File> file;
             ASSERT_NO_THROW(file = fs_->open("/dev/a/b/c/foo.txt"));
             ASSERT_THROW(fs_->unlink_directory("/dev/a/b/c"), sgfs::FileSystemException);
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
             ASSERT_NO_THROW(fs_->unlink_directory("/dev/a/b/c"));
             ASSERT_FALSE(fs_->directory_exists("/dev/a/b/c"));
         });
@@ -204,7 +204,7 @@ TEST_F(FileSystemTest, FileOpenClose)  {
             ASSERT_THROW(fs_->move_file("/dev/a/stuff/other.txt", "/dev/a/stuff/foo.txt"), sgfs::FileSystemException);
 
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
             XBT_INFO("Trying to unlink the file");
             ASSERT_NO_THROW(fs_->unlink_file("/dev/a/stuff/foo.txt"));
             ASSERT_FALSE(fs_->file_exists("/dev/a/stuff/foo.txt"));
@@ -217,7 +217,7 @@ TEST_F(FileSystemTest, FileOpenClose)  {
     });
 }
 
-TEST_F(FileSystemTest, TooManyFilesOpened)  {
+TEST_F(FileSystemTest, TooManyFilesOpened) {
     DO_TEST_WITH_FORK([this]() {
         this->setup_platform();
         // Create one actor (for this test we could likely do it all in the maestro but what the hell)
@@ -244,7 +244,7 @@ TEST_F(FileSystemTest, TooManyFilesOpened)  {
             XBT_INFO("Opening a third file, should not work");
             ASSERT_THROW(file3 = limited_fs->open("/dev/a/stuff/baz.txt"), sgfs::FileSystemException);
             XBT_INFO("Close the first file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(limited_fs->close(file));
             XBT_INFO("Opening a third file should now work");
             ASSERT_NO_THROW(file3 = limited_fs->open("/dev/a/stuff/baz.txt"));
         });

test/jbod_storage_test.cpp

@@ -105,7 +105,7 @@ TEST_F(JBODStorageTest, SingleRead)  {
             ASSERT_NO_THROW(file->seek(SEEK_SET));
             ASSERT_DOUBLE_EQ(file->read("9kB"), 9000);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -134,7 +134,7 @@ TEST_F(JBODStorageTest, SingleAsyncRead)  {
             XBT_INFO("Read complete. Clock should be at 2.1s (2s to read, 0.1 to transfer)");
             ASSERT_DOUBLE_EQ(sg4::Engine::get_clock(), 2.1);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -161,7 +161,7 @@ TEST_F(JBODStorageTest, SingleWrite)  {
             XBT_INFO("Check remaining space");
             ASSERT_DOUBLE_EQ(fs_->partition_by_name("/dev/a")->get_free_space(), 98 * 1000 * 1000);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -190,7 +190,7 @@ TEST_F(JBODStorageTest, SingleAsyncWrite)  {
             XBT_INFO("Write complete. Clock is at 4.12s (.1s to transfer, 0.02 to compute parity, 4s to write)");
             ASSERT_DOUBLE_EQ(sg4::Engine::get_clock(), 4.12);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -212,7 +212,7 @@ TEST_F(JBODStorageTest, ReadWriteUnsupportedRAID)  {
             XBT_INFO("Read 12MB at /dev/a/foo.txt, which should fail too");
             ASSERT_THROW(file->read("12MB"), std::runtime_error);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -239,7 +239,7 @@ TEST_F(JBODStorageTest, ReadWriteRAID0)  {
             XBT_INFO("Read complete. Clock is at 4.7s (1.5s to read, .1s to transfer)");
             ASSERT_DOUBLE_EQ(sg4::Engine::get_clock(), 4.7);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -266,7 +266,7 @@ TEST_F(JBODStorageTest, ReadWriteRAID1)  {
             XBT_INFO("Read complete. Clock is at 9.1s (3s to read, .05s to transfer)");
             ASSERT_DOUBLE_EQ(sg4::Engine::get_clock(), 9.1);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -293,7 +293,7 @@ TEST_F(JBODStorageTest, ReadWriteRAID4)  {
             XBT_INFO("Read complete. Clock is at 3.11s (1s to read, .05s to transfer)");
             ASSERT_DOUBLE_EQ(sg4::Engine::get_clock(), 3.11);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -338,7 +338,7 @@ TEST_F(JBODStorageTest, ReadWriteRAID6)  {
                 ASSERT_EQ(captured_debug_output, expected_debug_outputs.at(i));
             }
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());

test/one_disk_storage_test.cpp

@@ -60,7 +60,7 @@ TEST_F(OneDiskStorageTest, SingleRead)  {
             ASSERT_NO_THROW(file->seek(SEEK_SET));
             ASSERT_DOUBLE_EQ(file->read("9kB"), 9000);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -90,7 +90,7 @@ TEST_F(OneDiskStorageTest, SingleAsyncRead)  {
             ASSERT_DOUBLE_EQ(sg4::Engine::get_clock(), 2.0);
             ASSERT_DOUBLE_EQ(file->get_num_bytes_read(my_read), 4000000);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -117,7 +117,7 @@ TEST_F(OneDiskStorageTest, SingleWrite)  {
             XBT_INFO("Check remaining space");
             ASSERT_DOUBLE_EQ(fs_->partition_by_name("/dev/a")->get_free_space(), 98 * 1000 * 1000);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());
@@ -147,7 +147,7 @@ TEST_F(OneDiskStorageTest, SingleAsyncWrite)  {
             ASSERT_DOUBLE_EQ(sg4::Engine::get_clock(), 2.0);
             ASSERT_DOUBLE_EQ(file->get_num_bytes_written(my_write), 2000000);
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
         // Run the simulation
         ASSERT_NO_THROW(sg4::Engine::get_instance()->run());

test/seek_test.cpp

@@ -80,7 +80,7 @@ TEST_F(SeekTest, SeekandTell)  {
             XBT_INFO("Seek from an arbitrary position in file, should work");
             ASSERT_NO_THROW(file->seek(1000, 2000));
             XBT_INFO("Close the file");
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
             XBT_INFO("Checking file size of a non-existing file, which should fail");
             ASSERT_THROW(auto size = fs_->file_size("/dev/a/foo_bogus.txt"), sgfs::FileSystemException);
         });

test/stat_test.cpp

@@ -73,12 +73,12 @@ TEST_F(StatTest, Stat)  {
             ASSERT_DOUBLE_EQ(stat_struct->last_access_date, simgrid_get_clock());
             ASSERT_EQ(stat_struct->refcount, 2);
             XBT_INFO("Modifying file state");
-            ASSERT_NO_THROW(file2->close());
+            ASSERT_NO_THROW(fs_->close(file2));
             XBT_INFO("Calling stat() again");
             ASSERT_NO_THROW(stat_struct = file->stat());
             XBT_INFO("Checking sanity");
             ASSERT_EQ(stat_struct->refcount, 1);
-            ASSERT_NO_THROW(file->close());
+            ASSERT_NO_THROW(fs_->close(file));
         });
 
         // Run the simulation