From: Frederic Suter Date: Tue, 15 May 2018 08:45:58 +0000 (+0200) Subject: cleanups in Storage-related signals X-Git-Tag: v3.20~227 X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/e96187af67ea418609e2e4454afbab053b37fc2f cleanups in Storage-related signals --- diff --git a/include/simgrid/s4u/Storage.hpp b/include/simgrid/s4u/Storage.hpp index 3204e046ad..13ae9be296 100644 --- a/include/simgrid/s4u/Storage.hpp +++ b/include/simgrid/s4u/Storage.hpp @@ -57,11 +57,12 @@ public: surf::StorageImpl* getImpl() { return pimpl_; } /* The signals */ - /** @brief Callback signal fired when a new Link is created */ - static simgrid::xbt::signal onCreation; - - /** @brief Callback signal fired when a Link is destroyed */ - static simgrid::xbt::signal onDestruction; + /** @brief Callback signal fired when a new Storage is created */ + static simgrid::xbt::signal on_creation; + /** @brief Callback signal fired when a Storage is destroyed */ + static simgrid::xbt::signal on_destruction; + /** @brief Callback signal fired when a Storage's state changes */ + static simgrid::xbt::signal on_state_change; Host* attached_to_ = nullptr; diff --git a/src/kernel/EngineImpl.cpp b/src/kernel/EngineImpl.cpp index dcda57deda..ad6799fa1d 100644 --- a/src/kernel/EngineImpl.cpp +++ b/src/kernel/EngineImpl.cpp @@ -40,7 +40,7 @@ EngineImpl::~EngineImpl() for (auto const& kv : storages_) if (kv.second) - delete kv.second->getImpl(); + kv.second->getImpl()->destroy(); } } } diff --git a/src/plugins/file_system/s4u_FileSystem.cpp b/src/plugins/file_system/s4u_FileSystem.cpp index d89ecd0408..59574591c4 100644 --- a/src/plugins/file_system/s4u_FileSystem.cpp +++ b/src/plugins/file_system/s4u_FileSystem.cpp @@ -335,11 +335,6 @@ static void on_storage_creation(simgrid::s4u::Storage& st) st.extension_set(new FileSystemStorageExt(&st)); } -static void on_storage_destruction(simgrid::s4u::Storage& st) -{ - delete st.extension(); -} - static void on_host_creation(simgrid::s4u::Host& host) { host.extension_set(new FileDescriptorHostExt()); @@ -354,8 +349,7 @@ void sg_storage_file_system_init() if (not FileSystemStorageExt::EXTENSION_ID.valid()) { FileSystemStorageExt::EXTENSION_ID = simgrid::s4u::Storage::extension_create(); - simgrid::s4u::Storage::onCreation.connect(&on_storage_creation); - simgrid::s4u::Storage::onDestruction.connect(&on_storage_destruction); + simgrid::s4u::Storage::on_creation.connect(&on_storage_creation); } if (not FileDescriptorHostExt::EXTENSION_ID.valid()) { diff --git a/src/s4u/s4u_Storage.cpp b/src/s4u/s4u_Storage.cpp index 95e250c35a..a0e76998a5 100644 --- a/src/s4u/s4u_Storage.cpp +++ b/src/s4u/s4u_Storage.cpp @@ -83,8 +83,9 @@ sg_size_t Storage::write(sg_size_t size) /************* * Callbacks * *************/ -simgrid::xbt::signal Storage::onCreation; -simgrid::xbt::signal Storage::onDestruction; +simgrid::xbt::signal Storage::on_creation; +simgrid::xbt::signal Storage::on_destruction; +simgrid::xbt::signal Storage::on_state_change; } /* namespace s4u */ } /* namespace simgrid */ diff --git a/src/simix/smx_global.cpp b/src/simix/smx_global.cpp index f01bd8f961..a4fe1c11b3 100644 --- a/src/simix/smx_global.cpp +++ b/src/simix/smx_global.cpp @@ -214,9 +214,9 @@ void SIMIX_global_init(int *argc, char **argv) host.extension_set(new simgrid::simix::Host()); }); - simgrid::surf::storageCreatedCallbacks.connect([](simgrid::surf::StorageImpl* storage) { - sg_storage_t s = simgrid::s4u::Storage::byName(storage->get_cname()); - xbt_assert(s != nullptr, "Storage not found for name %s", storage->get_cname()); + simgrid::s4u::Storage::on_creation.connect([](simgrid::s4u::Storage& storage) { + sg_storage_t s = simgrid::s4u::Storage::byName(storage.get_cname()); + xbt_assert(s != nullptr, "Storage not found for name %s", storage.get_cname()); }); } diff --git a/src/surf/StorageImpl.cpp b/src/surf/StorageImpl.cpp index 999ec2a7a0..5df10ecc2f 100644 --- a/src/surf/StorageImpl.cpp +++ b/src/surf/StorageImpl.cpp @@ -16,16 +16,6 @@ simgrid::surf::StorageModel* surf_storage_model = nullptr; namespace simgrid { namespace surf { -/************* - * Callbacks * - *************/ - -simgrid::xbt::signal storageCreatedCallbacks; -simgrid::xbt::signal storageDestructedCallbacks; -simgrid::xbt::signal storageStateChangedCallbacks; // signature: wasOn, isOn -simgrid::xbt::signal - storageActionStateChangedCallbacks; - /********* * Model * *********/ @@ -62,7 +52,20 @@ StorageImpl::StorageImpl(kernel::resource::Model* model, std::string name, kerne StorageImpl::~StorageImpl() { - storageDestructedCallbacks(this); + xbt_assert(currentlyDestroying_, "Don't delete Storages directly. Call destroy() instead."); +} + +/** @brief Fire the required callbacks and destroy the object + * + * Don't delete directly a Storage, call s->destroy() instead. + */ +void StorageImpl::destroy() +{ + if (not currentlyDestroying_) { + currentlyDestroying_ = true; + s4u::Storage::on_destruction(this->piface_); + delete this; + } } bool StorageImpl::is_used() @@ -80,16 +83,18 @@ void StorageImpl::turn_on() { if (is_off()) { Resource::turn_on(); - storageStateChangedCallbacks(this, 0, 1); + s4u::Storage::on_state_change(this->piface_); } } void StorageImpl::turn_off() { if (is_on()) { Resource::turn_off(); - storageStateChangedCallbacks(this, 1, 0); + s4u::Storage::on_state_change(this->piface_); } } +xbt::signal + StorageAction::on_state_change; /********** * Action * @@ -98,7 +103,7 @@ void StorageAction::set_state(Action::State state) { Action::State old = get_state(); Action::set_state(state); - storageActionStateChangedCallbacks(this, old, state); + on_state_change(this, old, state); } } } diff --git a/src/surf/StorageImpl.hpp b/src/surf/StorageImpl.hpp index 29b98f895f..530c7287e9 100644 --- a/src/surf/StorageImpl.hpp +++ b/src/surf/StorageImpl.hpp @@ -29,25 +29,6 @@ class StorageAction; * Callbacks * *************/ -/** @ingroup SURF_callbacks - * @brief Callbacks handler which emit the callbacks after Storage creation * - * @details Callback functions have the following signature: `void(Storage*)` - */ -XBT_PUBLIC_DATA simgrid::xbt::signal storageCreatedCallbacks; - -/** @ingroup SURF_callbacks - * @brief Callbacks handler which emit the callbacks after Storage destruction * - * @details Callback functions have the following signature: `void(StoragePtr)` - */ -XBT_PUBLIC_DATA simgrid::xbt::signal storageDestructedCallbacks; - -/** @ingroup SURF_callbacks - * @brief Callbacks handler which emit the callbacks after Storage State changed * - * @details Callback functions have the following signature: `void(StorageAction *action, int previouslyOn, int - * currentlyOn)` - */ -XBT_PUBLIC_DATA simgrid::xbt::signal storageStateChangedCallbacks; - /** @ingroup SURF_callbacks * @brief Callbacks handler which emit the callbacks after StorageAction State changed * * @details Callback functions have the following signature: `void(StorageAction *action, @@ -55,7 +36,7 @@ XBT_PUBLIC_DATA simgrid::xbt::signal storageStateC */ XBT_PUBLIC_DATA simgrid::xbt::signal - storageActionStateChangedCallbacks; + on_state_change; /********* * Model * @@ -99,6 +80,8 @@ public: void turn_on() override; void turn_off() override; + void destroy(); // Must be called instead of the destructor + /** * @brief Read a file * @@ -124,6 +107,7 @@ public: sg_size_t size_; // Only used at parsing time then goes to the FileSystemExtension private: + bool currentlyDestroying_ = false; // Name of the host to which this storage is attached. Only used at platform parsing time, then the interface stores // the Host directly. std::string attach_; @@ -146,6 +130,9 @@ enum e_surf_action_storage_type_t { */ class StorageAction : public kernel::resource::Action { public: + static xbt::signal + on_state_change; + /** * @brief StorageAction constructor * diff --git a/src/surf/storage_n11.cpp b/src/surf/storage_n11.cpp index e1a0cd27b1..a36ca2e732 100644 --- a/src/surf/storage_n11.cpp +++ b/src/surf/storage_n11.cpp @@ -54,7 +54,6 @@ StorageImpl* StorageN11Model::createStorage(std::string id, std::string type_id, StorageImpl* storage = new StorageN11(this, id, get_maxmin_system(), Bread, Bwrite, type_id, content_name, storage_type->size, attach); - storageCreatedCallbacks(storage); XBT_DEBUG("SURF storage create resource\n\t\tid '%s'\n\t\ttype '%s'\n\t\tBread '%f'\n", id.c_str(), type_id.c_str(), Bread); @@ -93,7 +92,7 @@ StorageN11::StorageN11(StorageModel* model, std::string name, kernel::lmm::Syste : StorageImpl(model, name, maxminSystem, bread, bwrite, type_id, content_name, size, attach) { XBT_DEBUG("Create resource with Bread '%f' Bwrite '%f' and Size '%llu'", bread, bwrite, size); - simgrid::s4u::Storage::onCreation(this->piface_); + simgrid::s4u::Storage::on_creation(this->piface_); } StorageAction* StorageN11::read(sg_size_t size)