Merge branch 'v2.8' into v2.9

CHANGES/v2.8.md

@@ -113,4 +113,5 @@ Changes from version 2.7 which are relevant for users:
 - Small fix in Python, where we do not assume anymore that strings are null terminated.
 - Environment variables such as `PLUMED_INCLUDEDIR` and similar are sanitized before they are used for running commands.
 - Fixed a bug leading to a crash when using python selectors (e.g., `@mda:` or `@mdt:`) and multiple MPI processes per replica.
+- Fixed leaks in Subprocess, potentially givin problems with a large number (>250) of sequentially created plumed objects all of them using `@mda:` or `@mdt:` selectors.
 

regtest/basic/rt-make-subprocess/Makefile

@@ -0,0 +1 @@
+include ../../scripts/test.make

regtest/basic/rt-make-subprocess/config

@@ -0,0 +1,2 @@
+type=make
+plumed_needs=subprocess

regtest/basic/rt-make-subprocess/main.cpp

@@ -0,0 +1,31 @@
+#include "plumed/tools/Subprocess.h"
+#include <fstream>
+#include <unistd.h>
+
+using namespace PLMD;
+
+int main(){
+  // if processes are not properly terminated and cleaned
+  // only a finite number of them will be allowed
+
+  std::ofstream ofs("should_be_empty");
+
+  // the number of iteration required to trigger the bugs fixed in
+  // 2d45954bbb26471eaf48c72d9a9f0a2dcd8536cc
+  // are, on MacOS:
+  // ~ 250 for the missing close call
+  // ~ 2500 for the missing waitpid call
+
+  // notice that we are not spawning simultaneous processes.
+  // they are called in a sequential way and killed (by destructor)
+  for(unsigned i=0;i<10000;i++) {
+    try {
+      Subprocess sp("yes");
+    } catch(...) {
+      ofs<<"failed after "<<i<<"\n";
+      break;
+    }
+  }
+  return 0;
+}
+

src/tools/Subprocess.cpp

@@ -25,6 +25,7 @@
 #ifdef __PLUMED_HAS_SUBPROCESS
 #include <unistd.h>
 #include <csignal>
+#include <sys/wait.h>
 #endif
 
 namespace PLMD {
@@ -59,7 +60,11 @@ class SubprocessPid {
   }
   ~SubprocessPid() {
     // this is apparently working also with MPI on Travis.
-    if(pid!=0 && pid!=-1) kill(pid,SIGINT);
+    if(pid!=0 && pid!=-1) {
+      int status;
+      kill(pid,SIGINT);
+      waitpid(pid, &status, 0); // Wait for the child process to terminate
+    }
   }
 #endif
 };
@@ -113,11 +118,15 @@ Subprocess::Subprocess(const std::string & cmd) {
 
 Subprocess::~Subprocess() {
 #ifdef __PLUMED_HAS_SUBPROCESS
-// fpc should be closed to terminate the child executable
+// close files:
   fclose(fppc);
+  fclose(fpcp);
+// fclose also closes the underlying descriptors,
+// so this is not needed:
   close(fpc);
-// fcp should not be closed because it could make the child executable fail
-/// TODO: check if this is necessary and make this class exception safe!
+  close(fcp);
+// after closing the communication, the subprocess is killed
+// in pid's destructor
 #endif
 }