From 01f65054420344fa5207ecf2ade12ff4c81eabdf Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@esi-group.com>
Date: Wed, 1 Apr 2020 12:24:59 +0200
Subject: [PATCH] ENH: add a Pstream::shutdown() method (#1660)

- previously used a Pstream::exit() invoked from the argList
  destructor to handle all MPI shutdown, but this has the unfortunate
  side-effect of using a fixed return value for the program exit.

  Instead use the Pstream::shutdown() method in the destructor and allow
  the normal program exit codes as usual. This means that the
  following code now works as expected.

  ```
  argList args(...);

  if (...)
  {
      InfoErr<< "some error\n";
      return 1;
  }
  ```
---
 applications/test/error/Test-error.C          | 25 +++++++++++++------
 src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H | 13 +++++++---
 src/OpenFOAM/global/argList/parRun.H          | 16 +++++-------
 src/Pstream/dummy/UPstream.C                  |  8 ++++--
 src/Pstream/mpi/UPstream.C                    | 20 +++++++++------
 5 files changed, 51 insertions(+), 31 deletions(-)

diff --git a/applications/test/error/Test-error.C b/applications/test/error/Test-error.C
index e7bd245a6a0..550726330ba 100644
--- a/applications/test/error/Test-error.C
+++ b/applications/test/error/Test-error.C
@@ -6,6 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2011-2015 OpenFOAM Foundation
+    Copyright (C) 2020 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -27,17 +28,27 @@ Description
 
 \*---------------------------------------------------------------------------*/
 
-#include "error.H"
-#include "IOstreams.H"
+#include "argList.H"
 #include "dictionary.H"
 
 using namespace Foam;
 
 // * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * //
-// Main program:
 
 int main(int argc, char *argv[])
 {
+    #if 0
+    argList::noBanner();
+    argList args(argc, argv);
+
+    if (true)
+    {
+        InfoErr<< "Called with " << (args.size()-1) << " args\n";
+        InfoErr<< "... some error\n";
+        return 2;
+    }
+    #endif
+
     FatalError.throwExceptions();
 
     try
@@ -57,9 +68,9 @@ int main(int argc, char *argv[])
             << "Error 2"
             << exit(FatalError);
     }
-    catch (const Foam::error& fErr)
+    catch (const Foam::error& err)
     {
-        Serr<< "Caught Foam error " << fErr << nl << endl;
+        Serr<< "Caught Foam error " << err << nl << endl;
     }
 
     try
@@ -68,9 +79,9 @@ int main(int argc, char *argv[])
             << "Error# 3"
             << exit(FatalError);
     }
-    catch (const Foam::error& fErr)
+    catch (const Foam::error& err)
     {
-        Serr<< "Caught Foam error " << fErr << nl << endl;
+        Serr<< "Caught Foam error " << err << nl << endl;
     }
 
     return 0;
diff --git a/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H b/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H
index c3e7f6feb61..f963347395d 100644
--- a/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H
+++ b/src/OpenFOAM/db/IOstreams/Pstreams/UPstream.H
@@ -6,7 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2011-2017 OpenFOAM Foundation
-    Copyright (C) 2015-2016 OpenCFD Ltd.
+    Copyright (C) 2015-2020 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -385,6 +385,7 @@ public:
         //      Fatal if MPI has already been finalized.
         static bool initNull();
 
+
         // Non-blocking comms
 
             //- Get number of outstanding requests
@@ -510,12 +511,16 @@ public:
         }
 
 
-        //- Exit program
-        static void exit(int errnum = 1);
+        //- Shutdown (finalize) MPI as required.
+        //  Uses MPI_Abort instead of MPI_Finalize if errNo is non-zero
+        static void shutdown(int errNo = 0);
 
-        //- Abort program
+        //- Call MPI_Abort with no other checks or cleanup
         static void abort();
 
+        //- Shutdown (finalize) MPI as required and exit program with errNo.
+        static void exit(int errNo = 1);
+
         //- Exchange label with all processors (in the communicator).
         //  sendData[proci] is the label to send to proci.
         //  After return recvData contains the data from the other processors.
diff --git a/src/OpenFOAM/global/argList/parRun.H b/src/OpenFOAM/global/argList/parRun.H
index 42ee068b4aa..1c93d68db7d 100644
--- a/src/OpenFOAM/global/argList/parRun.H
+++ b/src/OpenFOAM/global/argList/parRun.H
@@ -6,7 +6,7 @@
      \\/     M anipulation  |
 -------------------------------------------------------------------------------
     Copyright (C) 2011-2018 OpenFOAM Foundation
-    Copyright (C) 2018 OpenCFD Ltd.
+    Copyright (C) 2018-2020 OpenCFD Ltd.
 -------------------------------------------------------------------------------
 License
     This file is part of OpenFOAM.
@@ -29,9 +29,7 @@ Class
 
 Description
     Helper class for initializing parallel jobs from the command arguments.
-
-    This class also handles cleanup of parallel or serial jobs in a
-    uniform manner.
+    Also handles cleanup of parallel (or serial) jobs.
 
 \*---------------------------------------------------------------------------*/
 
@@ -57,14 +55,14 @@ class ParRunControl
 
 public:
 
-    //- Construct null
+    //- Default construct
     ParRunControl()
     :
         parallel_(false),
         distributed_(false)
     {}
 
-    //- Destructor, triggers Pstream::exit
+    //- Destructor. Shutdown (finalize) MPI as required
     ~ParRunControl()
     {
         if (parallel_)
@@ -72,21 +70,19 @@ public:
             Info<< "Finalising parallel run" << endl;
         }
 
-        // Handles serial and parallel modes.
-        Pstream::exit(0);
+        Pstream::shutdown();
     }
 
 
     //- Initialize Pstream for a parallel run
     void runPar(int& argc, char**& argv, bool needsThread)
     {
-        parallel_ = true;
-
         if (!Pstream::init(argc, argv, needsThread))
         {
             Info<< "Failed to start parallel run" << endl;
             Pstream::exit(1);
         }
+        parallel_ = true;
     }
 
     //- True if this is parallel run.
diff --git a/src/Pstream/dummy/UPstream.C b/src/Pstream/dummy/UPstream.C
index cc7bcfd9402..54be585e85c 100644
--- a/src/Pstream/dummy/UPstream.C
+++ b/src/Pstream/dummy/UPstream.C
@@ -56,10 +56,14 @@ bool Foam::UPstream::init(int& argc, char**& argv, const bool needsThread)
 }
 
 
-void Foam::UPstream::exit(int errnum)
+void Foam::UPstream::shutdown(int errNo)
+{}
+
+
+void Foam::UPstream::exit(int errNo)
 {
     // No MPI - just exit
-    std::exit(errnum);
+    std::exit(errNo);
 }
 
 
diff --git a/src/Pstream/mpi/UPstream.C b/src/Pstream/mpi/UPstream.C
index 1a8f2ac6592..9eb2f370736 100644
--- a/src/Pstream/mpi/UPstream.C
+++ b/src/Pstream/mpi/UPstream.C
@@ -270,11 +270,11 @@ bool Foam::UPstream::init(int& argc, char**& argv, const bool needsThread)
 }
 
 
-void Foam::UPstream::exit(int errnum)
+void Foam::UPstream::shutdown(int errNo)
 {
     if (debug)
     {
-        Pout<< "UPstream::exit\n";
+        Pout<< "UPstream::shutdown\n";
     }
 
     int flag = 0;
@@ -282,8 +282,7 @@ void Foam::UPstream::exit(int errnum)
     MPI_Initialized(&flag);
     if (!flag)
     {
-        // Not initialized - just exit
-        std::exit(errnum);
+        // No MPI initialized - we are done
         return;
     }
 
@@ -298,7 +297,7 @@ void Foam::UPstream::exit(int errnum)
         }
         else if (debug)
         {
-            Pout<< "UPstream::exit : was already finalized\n";
+            Pout<< "UPstream::shutdown : was already finalized\n";
         }
     }
     else
@@ -352,17 +351,22 @@ void Foam::UPstream::exit(int errnum)
                 << "Finalizing MPI, but was initialized elsewhere\n";
         }
 
-        if (errnum == 0)
+        if (errNo == 0)
         {
             MPI_Finalize();
         }
         else
         {
-            MPI_Abort(MPI_COMM_WORLD, errnum);
+            MPI_Abort(MPI_COMM_WORLD, errNo);
         }
     }
+}
+
 
-    std::exit(errnum);
+void Foam::UPstream::exit(int errNo)
+{
+    UPstream::shutdown(errNo);
+    std::exit(errNo);
 }
 
 
-- 
GitLab