From 8bb78dda609dbbdfcf11f37851727cd47f01cab5 Mon Sep 17 00:00:00 2001
From: Mark Olesen <Mark.Olesen@Germany>
Date: Mon, 18 Jul 2016 13:37:39 +0200
Subject: [PATCH] ENH: provide a vfork/exec version of system (issue #185)
The normal library system() command uses 'fork', which causes
problems on IB+OPENMPI.
STYLE: add Foam:: qualifier to system calls to make them easier to spot.
---
.../foamHelp/helpTypes/helpType/helpType.C | 2 +-
.../dataConversion/foamToVTK/foamToVTK.C | 2 +-
.../viewFactorsGen/viewFactorsGen.C | 14 +-
src/OSspecific/POSIX/POSIX.C | 151 +++++++++++++++++-
.../dynamicLibrary/dynamicCode/dynamicCode.C | 15 +-
src/OpenFOAM/global/argList/argList.C | 2 +-
src/OpenFOAM/include/OSspecific.H | 11 +-
.../functionObjects/systemCall/controlDict | 9 +-
.../solarLoad/faceShading/faceShading.C | 14 +-
9 files changed, 197 insertions(+), 23 deletions(-)
diff --git a/applications/utilities/miscellaneous/foamHelp/helpTypes/helpType/helpType.C b/applications/utilities/miscellaneous/foamHelp/helpTypes/helpType/helpType.C
index 68ae82fa50a..a5b217e87ee 100644
--- a/applications/utilities/miscellaneous/foamHelp/helpTypes/helpType/helpType.C
+++ b/applications/utilities/miscellaneous/foamHelp/helpTypes/helpType/helpType.C
@@ -151,7 +151,7 @@ void Foam::helpType::displayDoc
Info<< "Source file: " << classDirectory.c_str() << classFile << nl
<< endl;
- system(docBrowser);
+ Foam::system(docBrowser);
}
else
{
diff --git a/applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK.C b/applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK.C
index 3ac7b6f7668..51a3563f5e1 100644
--- a/applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK.C
+++ b/applications/utilities/postProcessing/dataConversion/foamToVTK/foamToVTK.C
@@ -1309,7 +1309,7 @@ int main(int argc, char *argv[])
+ "_"
+ procFile.name()
);
- if (system(cmd.c_str()) == -1)
+ if (Foam::system(cmd.c_str()) == -1)
{
WarningInFunction
<< "Could not execute command " << cmd << endl;
diff --git a/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C b/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C
index 11a8becd5ad..a8e20b188f6 100644
--- a/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C
+++ b/applications/utilities/preProcessing/viewFactorsGen/viewFactorsGen.C
@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2016 OpenFOAM Foundation
- \\/ M anipulation |
+ \\/ M anipulation | Copyright (C) 2016 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
@@ -191,9 +191,15 @@ void writeRays
str << "l " << vertI-1 << ' ' << vertI << nl;
}
}
- string cmd("objToVTK " + fName + " " + fName.lessExt() + ".vtk");
- Pout<< "cmd:" << cmd << endl;
- system(cmd);
+ str.flush();
+
+ DynamicList<string> cmd(3);
+ cmd.append("objToVTK");
+ cmd.append(fName);
+ cmd.append(fName.lessExt() + ".vtk");
+
+ Pout<< "cmd: objToVTK " << fName.c_str() << endl;
+ Foam::system(cmd);
}
diff --git a/src/OSspecific/POSIX/POSIX.C b/src/OSspecific/POSIX/POSIX.C
index 0b9a541d940..8ecdaeb3e22 100644
--- a/src/OSspecific/POSIX/POSIX.C
+++ b/src/OSspecific/POSIX/POSIX.C
@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2016 OpenFOAM Foundation
- \\/ M anipulation |
+ \\/ M anipulation | Copyright (C) 2016 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
@@ -38,6 +38,8 @@ Description
#include "timer.H"
#include "IFstream.H"
#include "DynamicList.H"
+#include "CStringList.H"
+#include "SubList.H"
#include <fstream>
#include <cstdlib>
@@ -49,6 +51,7 @@ Description
#include <pwd.h>
#include <errno.h>
#include <sys/types.h>
+#include <sys/wait.h>
#include <sys/stat.h>
#include <sys/socket.h>
#include <netdb.h>
@@ -1166,9 +1169,153 @@ bool Foam::ping(const string& hostname, const label timeOut)
}
+namespace Foam
+{
+//! \cond fileScope
+static int waitpid(const pid_t pid)
+{
+ // child status, return code from the exec etc.
+ int status = 0;
+
+ // in parent - blocking wait
+ // modest treatment of signals (in child)
+ // treat 'stopped' like exit (suspend/continue)
+ while (true)
+ {
+ pid_t wpid = ::waitpid(pid, &status, WUNTRACED);
+
+ if (wpid == -1)
+ {
+ FatalErrorInFunction
+ << "some error occurred in child"
+ << exit(FatalError);
+ break;
+ }
+
+ if (WIFEXITED(status))
+ {
+ // child exited, get its return status
+ return WEXITSTATUS(status);
+ }
+
+ if (WIFSIGNALED(status))
+ {
+ // child terminated by some signal
+ return WTERMSIG(status);
+ }
+
+ if (WIFSTOPPED(status))
+ {
+ // child stopped by some signal
+ return WSTOPSIG(status);
+ }
+
+ FatalErrorInFunction
+ << "programming error, status from waitpid() not handled: "
+ << status
+ << exit(FatalError);
+ }
+
+ return -1; // should not happen
+}
+//! \endcond
+}
+
+
int Foam::system(const std::string& command)
{
- return ::system(command.c_str());
+ if (command.empty())
+ {
+ // Treat an empty command as a successful no-op.
+ // From 'man sh' POSIX (man sh):
+ // "If the command_string operand is an empty string,
+ // sh shall exit with a zero exit status."
+ return 0;
+ }
+
+ pid_t child_pid = ::vfork(); // NB: vfork, not fork!
+ if (child_pid == -1)
+ {
+ FatalErrorInFunction
+ << "vfork() failed for system command " << command
+ << exit(FatalError);
+ }
+
+ if (child_pid == 0)
+ {
+ // in child
+
+ // execl uses the current environ
+ (void) ::execl
+ (
+ "/bin/sh", // Path of the shell
+ "sh", // Command-name (name for the shell)
+ "-c", // Read commands from the command_string operand.
+ command.c_str(), // Command string
+ reinterpret_cast<char *>(0)
+ );
+
+ // obviously failed, since exec should not return at all
+ FatalErrorInFunction
+ << "exec failed: " << command
+ << exit(FatalError);
+ }
+
+
+ // in parent - blocking wait
+ return waitpid(child_pid);
+}
+
+
+int Foam::system(const Foam::UList<Foam::string>& command)
+{
+ const int argc = command.size();
+
+ if (!argc)
+ {
+ // Treat an empty command as a successful no-op.
+ // For consistency with POSIX (man sh) behaviour for (sh -c command),
+ // which is what is mostly being replicated here.
+ return 0;
+ }
+
+ // NB: use vfork, not fork!
+ // vfork behaves more like a thread and avoids copy-on-write problems
+ // triggered by fork.
+ // The normal system() command has a fork buried in it that causes
+ // issues with infiniband and openmpi etc.
+ pid_t child_pid = ::vfork();
+ if (child_pid == -1)
+ {
+ FatalErrorInFunction
+ << "vfork() failed for system command " << command[0]
+ << exit(FatalError);
+ }
+
+ if (child_pid == 0)
+ {
+ // in child:
+ // Need command and arguments separately.
+ // args is a NULL-terminated list of c-strings
+
+ CStringList args(SubList<string>(command, 0));
+ if (argc > 1)
+ {
+ args.reset(SubList<string>(command, argc-1, 1));
+ }
+
+ // execvp uses the current environ
+ (void) ::execvp(command[0].c_str(), args.strings());
+
+ // obviously failed, since exec should not return at all
+ FatalErrorInFunction
+ << "exec(" << command[0] << ", ...) failed"
+ << exit(FatalError);
+ }
+
+
+ // in parent - blocking wait
+ return waitpid(child_pid);
}
diff --git a/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C b/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C
index 04932fceea3..19882241e34 100644
--- a/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C
+++ b/src/OpenFOAM/db/dynamicLibrary/dynamicCode/dynamicCode.C
@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2015 OpenFOAM Foundation
- \\/ M anipulation |
+ \\/ M anipulation | Copyright (C) 2016 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
@@ -493,10 +493,17 @@ bool Foam::dynamicCode::copyOrCreateFiles(const bool verbose) const
bool Foam::dynamicCode::wmakeLibso() const
{
- const Foam::string wmakeCmd("wmake -s libso " + this->codePath());
- Info<< "Invoking " << wmakeCmd << endl;
+ DynamicList<string> cmd(4);
+ cmd.append("wmake");
+ cmd.append("-s");
+ cmd.append("libso");
+ cmd.append(this->codePath());
- if (Foam::system(wmakeCmd))
+ // NOTE: could also resolve wmake command explicitly
+ // cmd[0] = stringOps::expand("$WM_PROJECT_DIR/wmake/wmake");
+
+ Info<< "Invoking wmake libso " << this->codePath().c_str() << endl;
+ if (Foam::system(cmd))
{
return false;
}
diff --git a/src/OpenFOAM/global/argList/argList.C b/src/OpenFOAM/global/argList/argList.C
index 5d391afaafe..8f489589b02 100644
--- a/src/OpenFOAM/global/argList/argList.C
+++ b/src/OpenFOAM/global/argList/argList.C
@@ -1170,7 +1170,7 @@ void Foam::argList::displayDoc(bool source) const
Info<< "Show documentation: " << docBrowser.c_str() << endl;
- system(docBrowser);
+ Foam::system(docBrowser);
}
else
{
diff --git a/src/OpenFOAM/include/OSspecific.H b/src/OpenFOAM/include/OSspecific.H
index d190f67045e..4adc48aeeed 100644
--- a/src/OpenFOAM/include/OSspecific.H
+++ b/src/OpenFOAM/include/OSspecific.H
@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2011-2015 OpenFOAM Foundation
- \\/ M anipulation |
+ \\/ M anipulation | Copyright (C) 2016 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
@@ -37,6 +37,7 @@ SourceFiles
#define OSspecific_H
#include "fileNameList.H"
+#include "stringList.H"
#include <sys/types.h>
@@ -193,9 +194,15 @@ bool ping(const string&, const label port, const label timeOut);
//- Check if machine is up by pinging port 22 (ssh) and 222 (rsh)
bool ping(const string&, const label timeOut=10);
-//- Execute the specified command
+//- Execute the specified command via the shell.
+// Uses vfork/execl internally.
+// Where possible, use the list version instead.
int system(const std::string& command);
+//- Execute the specified command with arguments.
+// Uses vfork/execvp internally
+int system(const UList<string>& command);
+
//- Open a shared library. Return handle to library. Print error message
// if library cannot be loaded (check = true)
void* dlOpen(const fileName& lib, const bool check = true);
diff --git a/src/postProcessing/functionObjects/systemCall/controlDict b/src/postProcessing/functionObjects/systemCall/controlDict
index ff192e197cf..124c5406b9b 100644
--- a/src/postProcessing/functionObjects/systemCall/controlDict
+++ b/src/postProcessing/functionObjects/systemCall/controlDict
@@ -62,14 +62,15 @@ functions
// called at the end of the run
endCalls
(
- "echo \*\*\* writing .bashrc \*\*\*"
- "cat ~/.bashrc"
- "echo \*\*\* done \*\*\*"
+ // Note: single quotes to avoid shell expansion
+ "echo '*** listing ~/.bashrc ***'"
+ "cat ~/.bashrc; echo '*** done ***'"
);
- // called every ouput time
+ // called every output time
writeCalls
(
+ // Note: can also backslash to escape shell meta-characters
"echo \*\*\* writing data \*\*\*"
);
}
diff --git a/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C b/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C
index 6188b294681..d162635f342 100644
--- a/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C
+++ b/src/thermophysicalModels/radiation/radiationModels/solarLoad/faceShading/faceShading.C
@@ -3,7 +3,7 @@
\\ / F ield | OpenFOAM: The Open Source CFD Toolbox
\\ / O peration |
\\ / A nd | Copyright (C) 2015 OpenFOAM Foundation
- \\/ M anipulation |
+ \\/ M anipulation | Copyright (C) 2016 OpenCFD Ltd.
-------------------------------------------------------------------------------
License
This file is part of OpenFOAM.
@@ -62,9 +62,15 @@ void Foam::faceShading::writeRays
vertI++;
str << "l " << vertI-1 << ' ' << vertI << nl;
}
- string cmd("objToVTK " + fName + " " + fName.lessExt() + ".vtk");
- Pout<< "cmd:" << cmd << endl;
- system(cmd);
+ str.flush();
+
+ DynamicList<string> cmd(3);
+ cmd.append("objToVTK");
+ cmd.append(fName);
+ cmd.append(fName.lessExt() + ".vtk");
+
+ Pout<< "cmd: objToVTK " << fName.c_str() << endl;
+ Foam::system(cmd);
}
--
GitLab