MPI Win: add check if we try to delete a locked or opened window.

todo:
- refcount for wins,
- check if we need a separate counter for open/fence (going negative is hacky)

diff --git a/src/smpi/bindings/smpi_pmpi_win.cpp b/src/smpi/bindings/smpi_pmpi_win.cpp
index 26b6532..9c7e74d 100644 (file)
--- a/src/smpi/bindings/smpi_pmpi_win.cpp
+++ b/src/smpi/bindings/smpi_pmpi_win.cpp
@@ -105,7 +105,7 @@ int PMPI_Win_free( MPI_Win* win){
     return MPI_ERR_WIN;
   }
   const SmpiBenchGuard suspend_bench;
-  delete *win;
+  return simgrid::smpi::Win::del(*win);
   return MPI_SUCCESS;
 }
 

diff --git a/src/smpi/colls/alltoall/alltoall-pair.cpp b/src/smpi/colls/alltoall/alltoall-pair.cpp
index 57501a5..e991df0 100644 (file)
--- a/src/smpi/colls/alltoall/alltoall-pair.cpp
+++ b/src/smpi/colls/alltoall/alltoall-pair.cpp
@@ -58,7 +58,7 @@ int alltoall__pair_rma(const void *send_buff, int send_count, MPI_Datatype send_
             rank /* send_chunk*/, send_count, send_type);
   }
   win->fence(assert);
-  delete win;
+  simgrid::smpi::Win::del(win);
   return 0;
 }
 

diff --git a/src/smpi/include/smpi_win.hpp b/src/smpi/include/smpi_win.hpp
index 0789da5..339b3c2 100644 (file)
--- a/src/smpi/include/smpi_win.hpp
+++ b/src/smpi/include/smpi_win.hpp
@@ -53,7 +53,7 @@ public:
   Win(MPI_Info info, MPI_Comm comm) : Win(MPI_BOTTOM, 0, 1, info, comm, false, true){};
   Win(const Win&) = delete;
   Win& operator=(const Win&) = delete;
-  ~Win() override;
+  static int del(Win* win);
   int attach (void *base, MPI_Aint size);
   int detach (const void *base);
   void get_name(char* name, int* length) const;

diff --git a/src/smpi/mpi/smpi_file.cpp b/src/smpi/mpi/smpi_file.cpp
index db78594..5318322 100644 (file)
--- a/src/smpi/mpi/smpi_file.cpp
+++ b/src/smpi/mpi/smpi_file.cpp
@@ -81,7 +81,7 @@ File::~File()
     delete shared_file_pointer_;
     delete[] list_;
   }
-  delete win_;
+  simgrid::smpi::Win::del(win_);
   file_->close();
   F2C::free_f(this->f2c_id());
   if (info_ != MPI_INFO_NULL)

diff --git a/src/smpi/mpi/smpi_win.cpp b/src/smpi/mpi/smpi_win.cpp
index 186a758..0a34e11 100644 (file)
--- a/src/smpi/mpi/smpi_win.cpp
+++ b/src/smpi/mpi/smpi_win.cpp
@@ -77,29 +77,35 @@ Win::Win(void* base, MPI_Aint size, int disp_unit, MPI_Info info, MPI_Comm comm,
   this->add_f();
 }
 
-Win::~Win(){
+int Win::del(Win* win){
   //As per the standard, perform a barrier to ensure every async comm is finished
   if  (MC_is_active() || MC_record_replay_is_active())
-    bar_->wait();
+    win->bar_->wait();
   else
-    colls::barrier(comm_);
-  flush_local_all();
+    colls::barrier(win->comm_);
+  win->flush_local_all();
 
-  if (info_ != MPI_INFO_NULL)
-    simgrid::smpi::Info::unref(info_);
-  if (errhandler_ != MPI_ERRHANDLER_NULL)
-    simgrid::smpi::Errhandler::unref(errhandler_);
+  if (win->info_ != MPI_INFO_NULL)
+    simgrid::smpi::Info::unref(win->info_);
+  if (win->errhandler_ != MPI_ERRHANDLER_NULL)
+    simgrid::smpi::Errhandler::unref(win->errhandler_);
 
-  comm_->remove_rma_win(this);
+  win->comm_->remove_rma_win(win);
 
-  colls::barrier(comm_);
-  Comm::unref(comm_);
+  colls::barrier(win->comm_);
+  Comm::unref(win->comm_);
+  if (!win->lockers_.empty() || win->opened_ < 0){
+    XBT_WARN("Freeing a locked or opened window");
+    return MPI_ERR_WIN;
+  }
+  if (win->allocated_)
+    xbt_free(win->base_);
 
-  if (allocated_)
-    xbt_free(base_);
+  F2C::free_f(win->f2c_id());
+  win->cleanup_attr<Win>();
 
-  F2C::free_f(this->f2c_id());
-  cleanup_attr<Win>();
+  delete win;
+  return MPI_SUCCESS;
 }
 
 int Win::attach(void* /*base*/, MPI_Aint size)
@@ -434,7 +440,7 @@ int Win::start(MPI_Group group, int /*assert*/)
 
   group->ref();
   dst_group_ = group;
-  opened_++; // we're open for business !
+  opened_--; // we're open for business !
   XBT_DEBUG("Leaving MPI_Win_Start");
   return MPI_SUCCESS;
 }
@@ -459,7 +465,7 @@ int Win::post(MPI_Group group, int /*assert*/)
 
   group->ref();
   src_group_ = group;
-  opened_++; // we're open for business !
+  opened_--; // we're open for business !
   XBT_DEBUG("Leaving MPI_Win_Post");
   return MPI_SUCCESS;
 }
@@ -485,7 +491,7 @@ int Win::complete(){
 
   flush_local_all();
 
-  opened_--; //we're closed for business !
+  opened_++; //we're closed for business !
   Group::unref(dst_group_);
   dst_group_ = MPI_GROUP_NULL;
   return MPI_SUCCESS;
@@ -511,7 +517,7 @@ int Win::wait(){
 
   flush_local_all();
 
-  opened_--; //we're closed for business !
+  opened_++; //we're closed for business !
   Group::unref(src_group_);
   src_group_ = MPI_GROUP_NULL;
   return MPI_SUCCESS;