From 0c8dad94012cf9838d7bdb4b88b2d03843cf1c9b Mon Sep 17 00:00:00 2001
From: Martin Bauer <martin.bauer@fau.de>
Date: Thu, 13 Apr 2017 14:58:06 +0200
Subject: [PATCH] [Bugfix] BufferSystem was not copied correctly

- Bug occurred if a BufferSystem (or communication scheme) was copied
  and the first object went out of scope.
- Internally a pointer was kept which was still pointing to the original
  BufferSystem

Fixes #16
---
 src/core/mpi/BufferSystem.cpp       | 44 +++++++++++++++++++++++++++
 src/core/mpi/BufferSystem.h         |  2 ++
 src/core/mpi/BufferSystemHelper.h   | 11 +++----
 tests/core/mpi/BufferSystemTest.cpp | 46 +++++++++++++++++++++++++----
 4 files changed, 92 insertions(+), 11 deletions(-)

diff --git a/src/core/mpi/BufferSystem.cpp b/src/core/mpi/BufferSystem.cpp
index c67bd2af7..5659a874a 100644
--- a/src/core/mpi/BufferSystem.cpp
+++ b/src/core/mpi/BufferSystem.cpp
@@ -92,6 +92,50 @@ BufferSystem::BufferSystem( const MPI_Comm & communicator, int tag )
 {
 }
 
+
+BufferSystem::BufferSystem( const BufferSystem &other )
+   : knownSizeComm_  ( other.knownSizeComm_.getCommunicator(), other.knownSizeComm_.getTag() ),
+     unknownSizeComm_( other.knownSizeComm_.getCommunicator(), other.knownSizeComm_.getTag() ),
+     noMPIComm_      ( other.knownSizeComm_.getCommunicator(), other.knownSizeComm_.getTag() ),
+     currentComm_ ( NULL ),
+     sizeChangesEverytime_( other.sizeChangesEverytime_ ),
+     communicationRunning_( other.communicationRunning_ ),
+     recvInfos_( other.recvInfos_ ),
+     sendInfos_( other.sendInfos_ )
+{
+   WALBERLA_ASSERT( !communicationRunning_, "Can't copy BufferSystem while communication is running" );
+   if( other.currentComm_ == &other.knownSizeComm_ )
+      currentComm_ = &knownSizeComm_;
+   else if ( other.currentComm_ == &other.unknownSizeComm_ )
+      currentComm_ = &unknownSizeComm_;
+   else if ( other.currentComm_ == &other.noMPIComm_ )
+      currentComm_ = &noMPIComm_;
+   else
+      currentComm_ = NULL; // receiver information not yet set
+}
+
+
+BufferSystem & BufferSystem::operator=( const BufferSystem & other )
+{
+   WALBERLA_ASSERT( !communicationRunning_, "Can't copy BufferSystem while communication is running" );
+
+   sizeChangesEverytime_ = other.sizeChangesEverytime_;
+   communicationRunning_ = other.communicationRunning_;
+   recvInfos_ = other.recvInfos_;
+   sendInfos_ = other.sendInfos_;
+
+   if( other.currentComm_ == &other.knownSizeComm_ )
+      currentComm_ = &knownSizeComm_;
+   else if ( other.currentComm_ == &other.unknownSizeComm_ )
+      currentComm_ = &unknownSizeComm_;
+   else if ( other.currentComm_ == &other.noMPIComm_ )
+      currentComm_ = &noMPIComm_;
+   else
+      currentComm_ = NULL; // receiver information not yet set
+
+   return *this;
+}
+
 //======================================================================================================================
 //
 //  Receive Information Setup
diff --git a/src/core/mpi/BufferSystem.h b/src/core/mpi/BufferSystem.h
index 071520f8e..34042e56f 100644
--- a/src/core/mpi/BufferSystem.h
+++ b/src/core/mpi/BufferSystem.h
@@ -115,6 +115,8 @@ public:
    /*!\name Constructors */
    //@{
    explicit BufferSystem( const MPI_Comm & communicator, int tag = 0 );
+   BufferSystem( const BufferSystem & other );
+   BufferSystem & operator=( const BufferSystem & other );
    ~BufferSystem() {}
    //@}
    //*******************************************************************************************************************
diff --git a/src/core/mpi/BufferSystemHelper.h b/src/core/mpi/BufferSystemHelper.h
index 6991d1a58..b8e7a2928 100644
--- a/src/core/mpi/BufferSystemHelper.h
+++ b/src/core/mpi/BufferSystemHelper.h
@@ -90,7 +90,9 @@ namespace internal {
       virtual MPIRank waitForNextReceive( std::map<MPIRank, ReceiveInfo> & recvInfos ) = 0;
 
 
-       virtual int getTag() const { return tag_; }
+      virtual int getTag() const { return tag_; }
+
+      virtual MPI_Comm getCommunicator() const { return communicator_; }
 
    protected:
       MPI_Comm communicator_;
@@ -99,7 +101,6 @@ namespace internal {
 
 
 
-
    class KnownSizeCommunication : public AbstractCommunication
    {
    public:
@@ -126,7 +127,6 @@ namespace internal {
 
 
 
-
    class UnknownSizeCommunication : public AbstractCommunication
    {
    public:
@@ -138,7 +138,7 @@ namespace internal {
       virtual void send( MPIRank receiver, const SendBuffer & sendBuffer );
       virtual void waitForSends();
 
-      virtual void    scheduleReceives  ( std::map<MPIRank, ReceiveInfo> & recvInfos );
+      virtual void scheduleReceives( std::map<MPIRank, ReceiveInfo> & recvInfos );
 
       /// size field of recvInfos can be invalid, is filled in with the actual message size
       virtual MPIRank waitForNextReceive( std::map<MPIRank, ReceiveInfo> & recvInfos );
@@ -156,6 +156,7 @@ namespace internal {
    };
 
 
+
    class NoMPICommunication : public AbstractCommunication
    {
    public:
@@ -167,7 +168,7 @@ namespace internal {
       virtual void send( MPIRank receiver, const SendBuffer & sendBuffer );
       virtual void waitForSends();
 
-      virtual void    scheduleReceives( std::map<MPIRank, ReceiveInfo> & recvInfos );
+      virtual void scheduleReceives( std::map<MPIRank, ReceiveInfo> & recvInfos );
 
       /// size field of recvInfos can be invalid, is filled in with the actual message size
       virtual MPIRank waitForNextReceive( std::map<MPIRank, ReceiveInfo> & recvInfos );
diff --git a/tests/core/mpi/BufferSystemTest.cpp b/tests/core/mpi/BufferSystemTest.cpp
index bc270500b..9aa4d0f83 100644
--- a/tests/core/mpi/BufferSystemTest.cpp
+++ b/tests/core/mpi/BufferSystemTest.cpp
@@ -325,6 +325,38 @@ void selfSend()
    }
 }
 
+void copyTest()
+{
+   int rank = MPIManager::instance()->worldRank();
+
+   BufferSystem bs1( MPI_COMM_WORLD, 3 );
+   {
+      BufferSystem bs2( MPI_COMM_WORLD, 7 );
+      bs2.sendBuffer(rank) << int(42);
+      bs2.setReceiverInfoFromSendBufferState( true, false );
+      bs2.sendAll();
+
+      for ( auto i = bs2.begin(); i != bs2.end(); ++i )
+      {
+         int messageContent;
+         i.buffer() >> messageContent;
+         WALBERLA_CHECK_EQUAL(messageContent, 42);
+      }
+
+      bs1 = bs2;
+
+   }
+
+   bs1.sendBuffer(rank) << int(42);
+   bs1.sendAll();
+   for ( auto i = bs1.begin(); i != bs1.end(); ++i )
+   {
+      int messageContent;
+      i.buffer() >> messageContent;
+      WALBERLA_CHECK_EQUAL(messageContent, 42);
+   }
+
+}
 
 int main(int argc, char**argv)
 {
@@ -340,21 +372,23 @@ int main(int argc, char**argv)
       return 1;
    }
 
-   WALBERLA_ROOT_SECTION()  { WALBERLA_LOG_INFO("Testing Symmetric Communication..." ); }
+   WALBERLA_LOG_INFO_ON_ROOT("Testing Symmetric Communication...");
    symmetricCommunication();
 
-   WALBERLA_ROOT_SECTION()  { WALBERLA_LOG_INFO("Testing Asymmetric Communication..."); }
+   WALBERLA_LOG_INFO_ON_ROOT("Testing Asymmetric Communication...");
    asymmetricCommunication();
 
-   WALBERLA_ROOT_SECTION()  { WALBERLA_LOG_INFO("Testing time-varying Communication..."); }
+   WALBERLA_LOG_INFO_ON_ROOT("Testing time-varying Communication...");
    timeVaryingCommunication();
 
-   WALBERLA_ROOT_SECTION()  { WALBERLA_LOG_INFO("Testing Gather Operation..."); }
+   WALBERLA_LOG_INFO_ON_ROOT("Testing Gather Operation...");
    gatherUsingAsymmetricCommunication();
 
-   WALBERLA_ROOT_SECTION()  { WALBERLA_LOG_INFO("Testing selfsend..."); }
+   WALBERLA_LOG_INFO_ON_ROOT("Testing self-send...");
    selfSend();
 
+   WALBERLA_LOG_INFO_ON_ROOT("Testing Buffer System copy...");
+   copyTest();
+
    return EXIT_SUCCESS;
 }
-
-- 
GitLab