Fix memleaks with MPI_*_get_info, when info is duplicated.

[simgrid.git] / src / smpi / mpi / smpi_win.cpp

diff --git a/src/smpi/mpi/smpi_win.cpp b/src/smpi/mpi/smpi_win.cpp
index e63c5a8..38f901a 100644 (file)
--- a/src/smpi/mpi/smpi_win.cpp
+++ b/src/smpi/mpi/smpi_win.cpp
@@ -130,9 +130,6 @@ void Win::get_group(MPI_Group* group){
 
 MPI_Info Win::info()
 {
-  if (info_ == MPI_INFO_NULL)
-    info_ = new Info();
-  info_->ref();
   return info_;
 }
 
@@ -182,25 +179,12 @@ void Win::set_name(const char* name){
 int Win::fence(int assert)
 {
   XBT_DEBUG("Entering fence");
-  if (opened_ == 0)
-    opened_=1;
+  opened_++;
   if (not (assert & MPI_MODE_NOPRECEDE)) {
     // This is not the first fence => finalize what came before
     bar_->wait();
-    mut_->lock();
-    // This (simulated) mutex ensures that no process pushes to the vector of requests during the waitall.
-    // Without this, the vector could get redimensioned when another process pushes.
-    // This would result in the array used by Request::waitall() to be invalidated.
-    // Another solution would be to copy the data and cleanup the vector *before* Request::waitall
-
-    // start all requests that have been prepared by another process
-    if (not requests_.empty()) {
-      int size           = static_cast<int>(requests_.size());
-      MPI_Request* treqs = requests_.data();
-      Request::waitall(size, treqs, MPI_STATUSES_IGNORE);
-    }
+    flush_local_all();
     count_=0;
-    mut_->unlock();
   }
 
   if (assert & MPI_MODE_NOSUCCEED) // there should be no ops after this one, tell we are closed.
@@ -349,6 +333,9 @@ int Win::accumulate(const void *origin_addr, int origin_count, MPI_Datatype orig
     mut_->unlock();
   }
 
+  // FIXME: The current implementation fails to ensure the correct ordering of the accumulate requests.  The following
+  // 'flush' is a workaround to fix that.
+  flush(target_rank);
   XBT_DEBUG("Leaving MPI_Win_Accumulate");
   return MPI_SUCCESS;
 }
@@ -571,33 +558,36 @@ int Win::unlock_all(){
 }
 
 int Win::flush(int rank){
-  MPI_Win target_win = connected_wins_[rank];
-  int finished       = finish_comms(rank);
-  XBT_DEBUG("Win_flush on local %d - Finished %d RMA calls", rank, finished);
-  finished = target_win->finish_comms(rank_);
-  XBT_DEBUG("Win_flush on remote %d - Finished %d RMA calls", rank, finished);
+  int finished = finish_comms(rank);
+  XBT_DEBUG("Win_flush on local %d for remote %d - Finished %d RMA calls", rank_, rank, finished);
+  if (rank != rank_) {
+    finished = connected_wins_[rank]->finish_comms(rank_);
+    XBT_DEBUG("Win_flush on remote %d for local %d - Finished %d RMA calls", rank, rank_, finished);
+  }
   return MPI_SUCCESS;
 }
 
 int Win::flush_local(int rank){
   int finished = finish_comms(rank);
-  XBT_DEBUG("Win_flush_local for rank %d - Finished %d RMA calls", rank, finished);
+  XBT_DEBUG("Win_flush_local on local %d for remote %d - Finished %d RMA calls", rank_, rank, finished);
   return MPI_SUCCESS;
 }
 
 int Win::flush_all(){
   int finished = finish_comms();
-  XBT_DEBUG("Win_flush_all on local - Finished %d RMA calls", finished);
+  XBT_DEBUG("Win_flush_all on local %d - Finished %d RMA calls", rank_, finished);
   for (int i = 0; i < comm_->size(); i++) {
-    finished = connected_wins_[i]->finish_comms(rank_);
-    XBT_DEBUG("Win_flush_all on %d - Finished %d RMA calls", i, finished);
+    if (i != rank_) {
+      finished = connected_wins_[i]->finish_comms(rank_);
+      XBT_DEBUG("Win_flush_all on remote %d for local %d - Finished %d RMA calls", i, rank_, finished);
+    }
   }
   return MPI_SUCCESS;
 }
 
 int Win::flush_local_all(){
   int finished = finish_comms();
-  XBT_DEBUG("Win_flush_local_all - Finished %d RMA calls", finished);
+  XBT_DEBUG("Win_flush_local_all on local %d - Finished %d RMA calls", rank_, finished);
   return MPI_SUCCESS;
 }
 
@@ -606,6 +596,10 @@ Win* Win::f2c(int id){
 }
 
 int Win::finish_comms(){
+  // This (simulated) mutex ensures that no process pushes to the vector of requests during the waitall.
+  // Without this, the vector could get redimensioned when another process pushes.
+  // This would result in the array used by Request::waitall() to be invalidated.
+  // Another solution would be to copy the data and cleanup the vector *before* Request::waitall
   mut_->lock();
   //Finish own requests
   int size = static_cast<int>(requests_.size());
@@ -619,6 +613,7 @@ int Win::finish_comms(){
 }
 
 int Win::finish_comms(int rank){
+  // See comment about the mutex in finish_comms() above
   mut_->lock();
   // Finish own requests
   // Let's see if we're either the destination or the sender of this request