From: Augustin Degomme <adegomme@users.noreply.github.com>
Date: Fri, 19 Feb 2021 01:08:35 +0000 (+0100)
Subject: SMPI : add leak detection.
X-Git-Tag: v3.27~349
X-Git-Url: http://info.iut-bm.univ-fcomte.fr/pub/gitweb/simgrid.git/commitdiff_plain/8f29d9411160cd90ee8d2541c741daafb3f396c6

SMPI : add leak detection.
Use f2c mechanism used to convert handles to integers for fortran.
Warn when leaks are found at the end of the simulation.
Add option --cfg=smpi/list-leaks:n to display first n leaks to the user (we can only display pointer and type).
---

diff --git a/src/smpi/bindings/smpi_f77.cpp b/src/smpi/bindings/smpi_f77.cpp
index 0f6e37592c..0dbda6dcaf 100644
--- a/src/smpi/bindings/smpi_f77.cpp
+++ b/src/smpi/bindings/smpi_f77.cpp
@@ -77,6 +77,7 @@ void smpi_init_fortran_types()
 
     MPI_LB->add_f();
     MPI_UB->add_f();
+    simgrid::smpi::F2C::finish_initialization();
   }
 }
 
diff --git a/src/smpi/include/smpi_f2c.hpp b/src/smpi/include/smpi_f2c.hpp
index ac5d65b676..ef758dd38a 100644
--- a/src/smpi/include/smpi_f2c.hpp
+++ b/src/smpi/include/smpi_f2c.hpp
@@ -18,18 +18,19 @@ class F2C {
 private:
   // We use a single lookup table for every type.
   // Beware of collisions if id in mpif.h is not unique
-  static std::unordered_map<int, F2C*>* f2c_lookup_;
+  static std::unordered_map<unsigned int, F2C*>* f2c_lookup_;
   static int f2c_id_;
+  static std::unordered_map<unsigned int, F2C*>::size_type num_default_handles_;
   int my_f2c_id_ = -1;
 
 protected:
-  static void set_f2c_lookup(std::unordered_map<int, F2C*>* map) { f2c_lookup_ = map; }
+  static void set_f2c_lookup(std::unordered_map<unsigned int, F2C*>* map) { f2c_lookup_ = map; }
   static int f2c_id() { return f2c_id_; }
   static void f2c_id_increment() { f2c_id_++; }
 
 public:
-  static void delete_lookup() { delete f2c_lookup_; }
-  static std::unordered_map<int, F2C*>* lookup() { return f2c_lookup_; }
+  static void delete_lookup() { delete f2c_lookup_; f2c_lookup_ = nullptr ;}
+  static std::unordered_map<unsigned int, F2C*>* lookup() { return f2c_lookup_; }
   F2C();
   virtual ~F2C() = default;
 
@@ -41,6 +42,8 @@ public:
   // This method should be overridden in all subclasses to avoid casting the result when calling it.
   // For the default one, the MPI_*_NULL returned is assumed to be NULL.
   static F2C* f2c(int id);
+  static void finish_initialization() { num_default_handles_ = f2c_lookup_->size(); }
+  static std::unordered_map<unsigned int, F2C*>::size_type get_num_default_handles() { return num_default_handles_;}
 };
 
 }
diff --git a/src/smpi/include/smpi_op.hpp b/src/smpi/include/smpi_op.hpp
index a267f54adc..edf2042b47 100644
--- a/src/smpi/include/smpi_op.hpp
+++ b/src/smpi/include/smpi_op.hpp
@@ -20,7 +20,7 @@ class Op : public F2C{
   bool is_predefined_;
 
 public:
-  Op(MPI_User_function* function, bool commutative, bool predefined=false) : func_(function), is_commutative_(commutative), is_predefined_(predefined) {}
+  Op(MPI_User_function* function, bool commutative, bool predefined=false) : func_(function), is_commutative_(commutative), is_predefined_(predefined) {if(not predefined) this->add_f();}
   bool is_commutative() const { return is_commutative_; }
   bool is_predefined() const { return is_predefined_; }
   bool is_fortran_op() const { return is_fortran_op_; }
diff --git a/src/smpi/internals/smpi_config.cpp b/src/smpi/internals/smpi_config.cpp
index 2733ecd13a..ed6c33885b 100644
--- a/src/smpi/internals/smpi_config.cpp
+++ b/src/smpi/internals/smpi_config.cpp
@@ -182,6 +182,7 @@ void smpi_init_options(){
   if(_smpi_options_initialized)
     return;
   simgrid::config::declare_flag<bool>("smpi/display-timing", "Whether we should display the timing after simulation.", false);
+  simgrid::config::declare_flag<int>("smpi/list-leaks", "Whether we should display the n first MPI handle leaks (addresses and type only) after simulation", 0);
   simgrid::config::declare_flag<bool>("smpi/keep-temps", "Whether we should keep the generated temporary files.", false);
   simgrid::config::declare_flag<std::string>("smpi/tmpdir", "tmp dir for dlopen files", "/tmp");
 
diff --git a/src/smpi/internals/smpi_global.cpp b/src/smpi/internals/smpi_global.cpp
index ce1c397f4e..ceda03f0f6 100644
--- a/src/smpi/internals/smpi_global.cpp
+++ b/src/smpi/internals/smpi_global.cpp
@@ -20,6 +20,7 @@
 #include <array>
 #include <boost/algorithm/string.hpp> /* split */
 #include <boost/tokenizer.hpp>
+#include <boost/core/demangle.hpp>
 #include <cerrno>
 #include <cinttypes>
 #include <cstdint> /* intmax_t */
@@ -634,8 +635,24 @@ void SMPI_finalize()
 
   if (smpi_cfg_privatization() == SmpiPrivStrategies::MMAP)
     smpi_destroy_global_memory_segments();
-  if (simgrid::smpi::F2C::lookup() != nullptr)
+  if (simgrid::smpi::F2C::lookup() != nullptr){
+    if (simgrid::smpi::F2C::lookup()->size() > simgrid::smpi::F2C::get_num_default_handles()){
+      XBT_WARN("Probable Leaks in your code: SMPI detected %zu unfreed MPI handles : "
+               "display types and addresses (n max) with --cfg=smpi/list-leaks:n.\n"
+               "Running smpirun with -wrapper \"valgrind --leak-check=full\" can provide more information", 
+               simgrid::smpi::F2C::lookup()->size() - simgrid::smpi::F2C::get_num_default_handles());
+      int n = simgrid::config::get_value<int>("smpi/list-leaks");
+      if (n > 0) {
+        std::for_each(simgrid::smpi::F2C::lookup()->begin(),
+                  simgrid::smpi::F2C::lookup()->end(),
+                  [n](const std::pair<unsigned int, simgrid::smpi::F2C*> &p) {
+                      if(p.first >= simgrid::smpi::F2C::get_num_default_handles() && p.first < simgrid::smpi::F2C::get_num_default_handles() + n) 
+                        XBT_WARN ("Leak %p of type %s", p.second, boost::core::demangle(typeid(*(p.second)).name()).c_str() );
+                  });
+      }
+    }
     simgrid::smpi::F2C::delete_lookup();
+  }
 }
 
 void smpi_mpi_init() {
diff --git a/src/smpi/mpi/smpi_comm.cpp b/src/smpi/mpi/smpi_comm.cpp
index ce31cfaa83..5bcb35ee2e 100644
--- a/src/smpi/mpi/smpi_comm.cpp
+++ b/src/smpi/mpi/smpi_comm.cpp
@@ -34,6 +34,8 @@ Comm::Comm(MPI_Group group, MPI_Topology topo, bool smp, int in_id)
   errhandler_->ref();
   //First creation of comm is done before SIMIX_run, so only do comms for others
   if(in_id==MPI_UNDEFINED && smp==0 && this->rank()!=MPI_UNDEFINED ){
+    this->add_f();
+    group->add_f();
     int id;
     if(this->rank()==0){
       static int global_id_ = 0;
@@ -322,6 +324,8 @@ void Comm::unref(Comm* comm){
   Group::unref(comm->group_);
 
   if(comm->refcount_==0){
+    if(simgrid::smpi::F2C::lookup() != nullptr)
+      F2C::free_f(comm->c2f());
     comm->cleanup_smp();
     comm->cleanup_attr<Comm>();
     if (comm->info_ != MPI_INFO_NULL)
diff --git a/src/smpi/mpi/smpi_datatype.cpp b/src/smpi/mpi/smpi_datatype.cpp
index f16bf32c87..fa2c0dc6b3 100644
--- a/src/smpi/mpi/smpi_datatype.cpp
+++ b/src/smpi/mpi/smpi_datatype.cpp
@@ -37,10 +37,14 @@ static std::unordered_map<std::string, simgrid::smpi::Datatype*> id2type_lookup;
   const MPI_Datatype name = &_XBT_CONCAT(mpi_, name);
 
 // Predefined data types
+CREATE_MPI_DATATYPE_NULL(MPI_DATATYPE_NULL, -1)
+CREATE_MPI_DATATYPE(MPI_DOUBLE, 0, double)
+CREATE_MPI_DATATYPE(MPI_INT, 1, int)
 CREATE_MPI_DATATYPE(MPI_CHAR, 2, char)
 CREATE_MPI_DATATYPE(MPI_SHORT, 3, short)
-CREATE_MPI_DATATYPE(MPI_INT, 1, int)
 CREATE_MPI_DATATYPE(MPI_LONG, 4, long)
+CREATE_MPI_DATATYPE(MPI_FLOAT, 5, float)
+CREATE_MPI_DATATYPE(MPI_BYTE, 6, int8_t)
 CREATE_MPI_DATATYPE(MPI_LONG_LONG, 7, long long)
 CREATE_MPI_DATATYPE(MPI_SIGNED_CHAR, 8, signed char)
 CREATE_MPI_DATATYPE(MPI_UNSIGNED_CHAR, 9, unsigned char)
@@ -48,12 +52,9 @@ CREATE_MPI_DATATYPE(MPI_UNSIGNED_SHORT, 10, unsigned short)
 CREATE_MPI_DATATYPE(MPI_UNSIGNED, 11, unsigned int)
 CREATE_MPI_DATATYPE(MPI_UNSIGNED_LONG, 12, unsigned long)
 CREATE_MPI_DATATYPE(MPI_UNSIGNED_LONG_LONG, 13, unsigned long long)
-CREATE_MPI_DATATYPE(MPI_FLOAT, 5, float)
-CREATE_MPI_DATATYPE(MPI_DOUBLE, 0, double)
 CREATE_MPI_DATATYPE(MPI_LONG_DOUBLE, 14, long double)
 CREATE_MPI_DATATYPE(MPI_WCHAR, 15, wchar_t)
 CREATE_MPI_DATATYPE(MPI_C_BOOL, 16, bool)
-CREATE_MPI_DATATYPE(MPI_BYTE, 6, int8_t)
 CREATE_MPI_DATATYPE(MPI_INT8_T, 17, int8_t)
 CREATE_MPI_DATATYPE(MPI_INT16_T, 18, int16_t)
 CREATE_MPI_DATATYPE(MPI_INT32_T, 19, int32_t)
@@ -81,7 +82,6 @@ CREATE_MPI_DATATYPE(MPI_REAL, 38, float)
 CREATE_MPI_DATATYPE(MPI_REAL4, 39, float)
 CREATE_MPI_DATATYPE(MPI_REAL8, 40, double)
 CREATE_MPI_DATATYPE(MPI_REAL16, 41, long double)
-CREATE_MPI_DATATYPE_NULL(MPI_DATATYPE_NULL, -1)
 CREATE_MPI_DATATYPE(MPI_COMPLEX8, 42, float_float)
 CREATE_MPI_DATATYPE(MPI_COMPLEX16, 43, double_double)
 CREATE_MPI_DATATYPE(MPI_COMPLEX32, 44, double_double)
@@ -99,7 +99,7 @@ CREATE_MPI_DATATYPE(MPI_PACKED, 53, char)
 // Internal use only
 CREATE_MPI_DATATYPE(MPI_PTR, 54, void*)
 CREATE_MPI_DATATYPE(MPI_COUNT, 55, long long)
-
+#define NUM_BASIC_DATATYPES 57
 
 namespace simgrid{
 namespace smpi{
@@ -113,6 +113,7 @@ Datatype::Datatype(int ident, int size, MPI_Aint lb, MPI_Aint ub, int flags) : D
 
 Datatype::Datatype(int size, MPI_Aint lb, MPI_Aint ub, int flags) : size_(size), lb_(lb), ub_(ub), flags_(flags)
 {
+  this->add_f();
 #if SIMGRID_HAVE_MC
   if(MC_is_active())
     MC_ignore(&(refcount_), sizeof(refcount_));
@@ -133,6 +134,7 @@ Datatype::Datatype(const char* name, int ident, int size, MPI_Aint lb, MPI_Aint
 Datatype::Datatype(Datatype* datatype, int* ret)
     : size_(datatype->size_), lb_(datatype->lb_), ub_(datatype->ub_), flags_(datatype->flags_)
 {
+  this->add_f();
   *ret = this->copy_attrs(datatype);
 }
 
@@ -144,12 +146,12 @@ Datatype::~Datatype()
     return;
   //prevent further usage
   flags_ &= ~ DT_FLAG_COMMITED;
+  F2C::free_f(this->c2f());
   //if still used, mark for deletion
   if(refcount_!=0){
       flags_ |=DT_FLAG_DESTROYED;
       return;
   }
-
   cleanup_attr<Datatype>();
   delete contents_;
 }
@@ -646,5 +648,6 @@ Datatype* Datatype::f2c(int id)
 {
   return static_cast<Datatype*>(F2C::f2c(id));
 }
+
 } // namespace smpi
 } // namespace simgrid
diff --git a/src/smpi/mpi/smpi_f2c.cpp b/src/smpi/mpi/smpi_f2c.cpp
index 680c850e98..ff101983d0 100644
--- a/src/smpi/mpi/smpi_f2c.cpp
+++ b/src/smpi/mpi/smpi_f2c.cpp
@@ -15,8 +15,9 @@ int mpi_statuses_ignore_;
 namespace simgrid{
 namespace smpi{
 
-std::unordered_map<int, F2C*>* F2C::f2c_lookup_ = nullptr;
+std::unordered_map<unsigned int, F2C*>* F2C::f2c_lookup_ = nullptr;
 int F2C::f2c_id_ = 0;
+std::unordered_map<unsigned int, F2C*>::size_type F2C::num_default_handles_ = 0;
 
 // Keep it non trivially-constructible, or it will break MC+smpi on FreeBSD with Clang (don't ask why)
 F2C::F2C() = default;
@@ -24,7 +25,7 @@ F2C::F2C() = default;
 int F2C::add_f()
 {
   if (f2c_lookup_ == nullptr)
-    f2c_lookup_ = new std::unordered_map<int, F2C*>();
+    f2c_lookup_ = new std::unordered_map<unsigned int, F2C*>();
 
   my_f2c_id_                 = f2c_id();
   (*f2c_lookup_)[my_f2c_id_] = this;
@@ -35,7 +36,7 @@ int F2C::add_f()
 int F2C::c2f()
 {
   if (f2c_lookup_ == nullptr) {
-    f2c_lookup_ = new std::unordered_map<int, F2C*>();
+    f2c_lookup_ = new std::unordered_map<unsigned int, F2C*>();
   }
 
   if(my_f2c_id_==-1)
@@ -48,7 +49,7 @@ int F2C::c2f()
 F2C* F2C::f2c(int id)
 {
   if (f2c_lookup_ == nullptr)
-    f2c_lookup_ = new std::unordered_map<int, F2C*>();
+    f2c_lookup_ = new std::unordered_map<unsigned int, F2C*>();
 
   if(id >= 0){
     auto comm = f2c_lookup_->find(id);
diff --git a/src/smpi/mpi/smpi_file.cpp b/src/smpi/mpi/smpi_file.cpp
index 5d0229bb14..e5f00dd429 100644
--- a/src/smpi/mpi/smpi_file.cpp
+++ b/src/smpi/mpi/smpi_file.cpp
@@ -71,6 +71,7 @@ namespace smpi{
     simgrid::smpi::colls::bcast(&shared_mutex_, 1, MPI_AINT, 0, comm);
     if(comm_->rank() != 0)
       intrusive_ptr_add_ref(&*shared_mutex_);
+    this->add_f();
   }
 
   File::~File(){
@@ -80,6 +81,7 @@ namespace smpi{
     }
     delete win_;
     delete file_;
+    F2C::free_f(this->c2f());
     if (info_ != MPI_INFO_NULL)
       simgrid::smpi::Info::unref(info_);
     if (errhandler_ != MPI_ERRHANDLER_NULL)
diff --git a/src/smpi/mpi/smpi_group.cpp b/src/smpi/mpi/smpi_group.cpp
index d6eec8f96d..977b1c4161 100644
--- a/src/smpi/mpi/smpi_group.cpp
+++ b/src/smpi/mpi/smpi_group.cpp
@@ -10,6 +10,7 @@
 
 simgrid::smpi::Group mpi_MPI_GROUP_EMPTY;
 MPI_Group MPI_GROUP_EMPTY=&mpi_MPI_GROUP_EMPTY;
+extern XBT_PRIVATE MPI_Comm MPI_COMM_UNINITIALIZED;
 
 namespace simgrid{
 namespace smpi{
@@ -77,6 +78,8 @@ void Group::unref(Group* group)
 {
   group->refcount_--;
   if (group->refcount_ <= 0) {
+    if (simgrid::smpi::F2C::lookup() != nullptr)
+      F2C::free_f(group->c2f());
     delete group;
   }
 }
@@ -118,6 +121,7 @@ int Group::incl(int n, const int* ranks, MPI_Group* newgroup)
       s4u::Actor* actor = this->actor(ranks[i]); // ranks[] was passed as a param!
       (*newgroup)->set_mapping(actor, i);
     }
+    (*newgroup)->add_f();
   }
   return MPI_SUCCESS;
 }
@@ -146,6 +150,7 @@ int Group::group_union(MPI_Group group2, MPI_Group* newgroup)
       s4u::Actor* actor = group2->actor(i - size2);
       (*newgroup)->set_mapping(actor, i);
     }
+    (*newgroup)->add_f();
   }
   return MPI_SUCCESS;
 }
@@ -172,6 +177,7 @@ int Group::intersection(MPI_Group group2, MPI_Group* newgroup)
         (*newgroup)->set_mapping(actor, j);
         j++;
       }
+      (*newgroup)->add_f();
     }
   }
   return MPI_SUCCESS;
@@ -199,6 +205,7 @@ int Group::difference(MPI_Group group2, MPI_Group* newgroup)
         (*newgroup)->set_mapping(actor, i);
       }
     }
+    (*newgroup)->add_f();
   }
   return MPI_SUCCESS;
 }
@@ -218,6 +225,7 @@ int Group::excl(int n, const int *ranks, MPI_Group * newgroup){
       j++;
     }
   }
+  (*newgroup)->add_f();
   return MPI_SUCCESS;
 }
 
@@ -261,6 +269,7 @@ int Group::range_incl(int n, int ranges[][3], MPI_Group * newgroup){
         break;
     }
   }
+  (*newgroup)->add_f();
   return MPI_SUCCESS;
 }
 
@@ -309,6 +318,7 @@ int Group::range_excl(int n, int ranges[][3], MPI_Group * newgroup){
       oldrank++;
     }
   }
+  (*newgroup)->add_f();
   return MPI_SUCCESS;
 }
 
diff --git a/src/smpi/mpi/smpi_op.cpp b/src/smpi/mpi/smpi_op.cpp
index f2e3d7c25e..b67f40f799 100644
--- a/src/smpi/mpi/smpi_op.cpp
+++ b/src/smpi/mpi/smpi_op.cpp
@@ -270,8 +270,10 @@ void Op::ref(){
 void Op::unref(MPI_Op* op){
   if((*op)!=MPI_OP_NULL){
     (*op)->refcount_--;
-    if ((*op)->refcount_ == 0 && not (*op)->is_predefined_)
+    if ((*op)->refcount_ == 0 && not (*op)->is_predefined_){
+      F2C::free_f((*op)->c2f());
       delete(*op);
+    }
   }
 }
 
diff --git a/src/smpi/mpi/smpi_request.cpp b/src/smpi/mpi/smpi_request.cpp
index b250536da4..8467be601e 100644
--- a/src/smpi/mpi/smpi_request.cpp
+++ b/src/smpi/mpi/smpi_request.cpp
@@ -65,6 +65,7 @@ Request::Request(const void* buf, int count, MPI_Datatype datatype, int src, int
   nbc_requests_=nullptr;
   nbc_requests_size_=0;
   init_buffer(count);
+  this->add_f();
 }
 
 void Request::ref(){
@@ -90,6 +91,7 @@ void Request::unref(MPI_Request* request)
         Op::unref(&(*request)->op_);
 
       (*request)->print_request("Destroying");
+      F2C::free_f((*request)->c2f());
       delete *request;
       *request = MPI_REQUEST_NULL;
     }else{
diff --git a/src/smpi/mpi/smpi_win.cpp b/src/smpi/mpi/smpi_win.cpp
index 6c95f11d93..ca86d3f265 100644
--- a/src/smpi/mpi/smpi_win.cpp
+++ b/src/smpi/mpi/smpi_win.cpp
@@ -52,6 +52,7 @@ Win::Win(void* base, MPI_Aint size, int disp_unit, MPI_Info info, MPI_Comm comm,
   colls::bcast(&(bar_), sizeof(s4u::Barrier*), MPI_BYTE, 0, comm);
 
   colls::barrier(comm);
+  this->add_f();
 }
 
 Win::~Win(){
@@ -77,6 +78,7 @@ Win::~Win(){
   if(allocated_ !=0)
     xbt_free(base_);
 
+  F2C::free_f(this->c2f());
   cleanup_attr<Win>();
 }