diff --git a/examples/jbod.cpp b/examples/jbod.cpp index f9aeaed..827a1c7 100644 --- a/examples/jbod.cpp +++ b/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_)); } }; diff --git a/examples/open_seek_write.cpp b/examples/open_seek_write.cpp index 4e1289b..81f907a 100644 --- a/examples/open_seek_write.cpp +++ b/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_)); } }; diff --git a/include/fsmod/File.hpp b/include/fsmod/File.hpp index ab156e0..4b32adf 100644 --- a/include/fsmod/File.hpp +++ b/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 stat() const; }; diff --git a/include/fsmod/FileSystem.hpp b/include/fsmod/FileSystem.hpp index 77179f7..e14b931 100644 --- a/include/fsmod/FileSystem.hpp +++ b/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 open(const std::string& full_path); + void close(std::shared_ptr f); [[nodiscard]] std::shared_ptr partition_by_name(const std::string& name) const; [[nodiscard]] std::shared_ptr partition_by_name_or_null(const std::string& name) const; diff --git a/src/File.cpp b/src/File.cpp index 26eab92..98b3925 100644 --- a/src/File.cpp +++ b/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 ( diff --git a/src/FileSystem.cpp b/src/FileSystem.cpp index 9f71691..629c848 100644 --- a/src/FileSystem.cpp +++ b/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) { + this->num_open_files_--; + file->metadata_->decrease_file_refcount(); + } + /** * @brief * @param full_path: an absolute file path diff --git a/test/caching_test.cpp b/test/caching_test.cpp index 289e7cf..18a5035 100644 --- a/test/caching_test.cpp +++ b/test/caching_test.cpp @@ -153,7 +153,7 @@ TEST_F(CachingTest, LRUBasics) { std::shared_ptr 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) { diff --git a/test/file_system_test.cpp b/test/file_system_test.cpp index 64f6d81..9a80636 100644 --- a/test/file_system_test.cpp +++ b/test/file_system_test.cpp @@ -131,7 +131,7 @@ TEST_F(FileSystemTest, Directories) { std::shared_ptr 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")); }); diff --git a/test/jbod_storage_test.cpp b/test/jbod_storage_test.cpp index 3516fde..c4dc9aa 100644 --- a/test/jbod_storage_test.cpp +++ b/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()); diff --git a/test/one_disk_storage_test.cpp b/test/one_disk_storage_test.cpp index ffcdad7..7e1b320 100644 --- a/test/one_disk_storage_test.cpp +++ b/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()); diff --git a/test/seek_test.cpp b/test/seek_test.cpp index e3e3885..f182c95 100644 --- a/test/seek_test.cpp +++ b/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); }); diff --git a/test/stat_test.cpp b/test/stat_test.cpp index f7d2b9d..c046d54 100644 --- a/test/stat_test.cpp +++ b/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