From 130f51aeb55bd3bf2706c1f5b09ca59fa291c087 Mon Sep 17 00:00:00 2001 From: Martin Quinson Date: Tue, 14 Nov 2023 20:39:23 +0100 Subject: [PATCH] Have create_jbod() return a JbodPtr instead of Jbod* to avoid memleaks --- examples/cpp/plugin-jbod/s4u-plugin-jbod.cpp | 22 +++++++-------- include/simgrid/plugins/jbod.hpp | 28 +++++++++++++++----- src/plugins/jbod.cpp | 6 ++--- 3 files changed, 36 insertions(+), 20 deletions(-) diff --git a/examples/cpp/plugin-jbod/s4u-plugin-jbod.cpp b/examples/cpp/plugin-jbod/s4u-plugin-jbod.cpp index a3cb9e800e..bb3b20b171 100644 --- a/examples/cpp/plugin-jbod/s4u-plugin-jbod.cpp +++ b/examples/cpp/plugin-jbod/s4u-plugin-jbod.cpp @@ -9,7 +9,7 @@ XBT_LOG_NEW_DEFAULT_CATEGORY(jbod_test, "Messages specific for this simulation"); namespace sg4 = simgrid::s4u; -static void write_then_read(simgrid::plugin::Jbod* jbod) +static void write_then_read(simgrid::plugin::JbodPtr jbod) { simgrid::plugin::JbodIoPtr io = jbod->write_async(1e7); XBT_INFO("asynchronous write posted, wait for it"); @@ -39,24 +39,24 @@ int main(int argc, char** argv) // set up link so that data transfer from host to JBOD takes exactly 1 second (without crosstraffic) auto* link = zone->create_link("link", 1e7/0.97)->set_latency(0); - auto* jbod_raid0 = - simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid0", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID0, 1e7, 5e6); + auto jbod_raid0 = + simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid0", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID0, 1e7, 5e6); zone->add_route(host, jbod_raid0->get_controller(), {link}); - auto* jbod_raid1 = - simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid1", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID1, 1e7, 5e6); + auto jbod_raid1 = + simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid1", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID1, 1e7, 5e6); zone->add_route(host, jbod_raid1->get_controller(), {link}); - auto* jbod_raid4 = - simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid4", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID4, 1e7, 5e6); + auto jbod_raid4 = + simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid4", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID4, 1e7, 5e6); zone->add_route(host, jbod_raid4->get_controller(), {link}); - auto* jbod_raid5 = - simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid5", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID5, 1e7, 5e6); + auto jbod_raid5 = + simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid5", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID5, 1e7, 5e6); zone->add_route(host, jbod_raid5->get_controller(), {link}); - auto* jbod_raid6 = - simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid6", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID6, 1e7, 5e6); + auto jbod_raid6 = + simgrid::plugin::Jbod::create_jbod(zone, "jbod_raid6", 1e9, 4, simgrid::plugin::Jbod::RAID::RAID6, 1e7, 5e6); zone->add_route(host, jbod_raid6->get_controller(), {link}); zone->seal(); diff --git a/include/simgrid/plugins/jbod.hpp b/include/simgrid/plugins/jbod.hpp index 4ee799f552..202ec3c45e 100644 --- a/include/simgrid/plugins/jbod.hpp +++ b/include/simgrid/plugins/jbod.hpp @@ -10,11 +10,10 @@ namespace simgrid::plugin { +class Jbod; +using JbodPtr = boost::intrusive_ptr; class JbodIo; -/** Smart pointer to a simgrid::s4u::Activity */ using JbodIoPtr = boost::intrusive_ptr; -XBT_PUBLIC void intrusive_ptr_release(const JbodIo* io); -XBT_PUBLIC void intrusive_ptr_add_ref(const JbodIo* io); class Jbod { public: @@ -31,8 +30,8 @@ public: JbodIoPtr write_async(sg_size_t size); sg_size_t write(sg_size_t size); - static Jbod* create_jbod(s4u::NetZone* zone, const std::string& name, double speed, unsigned int num_disks, - RAID raid_level, double read_bandwidth, double write_bandwidth); + static JbodPtr create_jbod(s4u::NetZone* zone, const std::string& name, double speed, unsigned int num_disks, + RAID raid_level, double read_bandwidth, double write_bandwidth); protected: void set_controller(s4u::Host* host) { controller_ = host; } @@ -47,6 +46,17 @@ private: RAID raid_level_; unsigned int parity_disk_idx_; int read_disk_idx_; + std::atomic_int_fast32_t refcount_{1}; +#ifndef DOXYGEN + friend void intrusive_ptr_release(Jbod* jbod) + { + if (jbod->refcount_.fetch_sub(1, std::memory_order_release) == 1) { + std::atomic_thread_fence(std::memory_order_acquire); + delete jbod; + } + } + friend void intrusive_ptr_add_ref(Jbod* jbod) { jbod->refcount_.fetch_add(1, std::memory_order_relaxed); } +#endif }; class JbodIo { @@ -77,5 +87,11 @@ public: #endif }; +/* Refcounting functions */ +XBT_PUBLIC void intrusive_ptr_release(const Jbod* io); +XBT_PUBLIC void intrusive_ptr_add_ref(const Jbod* io); +XBT_PUBLIC void intrusive_ptr_release(const JbodIo* io); +XBT_PUBLIC void intrusive_ptr_add_ref(const JbodIo* io); + } // namespace simgrid::plugin -#endif \ No newline at end of file +#endif diff --git a/src/plugins/jbod.cpp b/src/plugins/jbod.cpp index b6f87c2be0..ee767fba6f 100644 --- a/src/plugins/jbod.cpp +++ b/src/plugins/jbod.cpp @@ -14,8 +14,8 @@ XBT_LOG_NEW_DEFAULT_SUBCATEGORY(s4u_jbod, s4u, "Logging specific to the JBOD imp namespace simgrid::plugin { -Jbod* Jbod::create_jbod(s4u::NetZone* zone, const std::string& name, double speed, unsigned int num_disks, - RAID raid_level, double read_bandwidth, double write_bandwidth) +JbodPtr Jbod::create_jbod(s4u::NetZone* zone, const std::string& name, double speed, unsigned int num_disks, + RAID raid_level, double read_bandwidth, double write_bandwidth) { xbt_assert(not ((raid_level == RAID::RAID4 || raid_level == RAID::RAID5) && num_disks < 3), "RAID%d requires at least 3 disks", (int) raid_level); @@ -30,7 +30,7 @@ Jbod* Jbod::create_jbod(s4u::NetZone* zone, const std::string& name, double spee for (unsigned int i = 0; i < num_disks; i++) jbod->get_controller()->create_disk(name + "_disk_" + std::to_string(i), read_bandwidth, write_bandwidth); - return jbod; + return JbodPtr(jbod); } JbodIoPtr Jbod::read_async(sg_size_t size) -- 2.20.1