wait for procdump to exit too. (#7731)

* wait for procdump to exit too.

* move running process check up, propagate status. First check whether processes are still running before attempting to force kill them

* only list processes, don't change their state

* add stopping procdump in all places

UnitTests/unittest.js

@@ -268,6 +268,20 @@ function main (argv) {
     crashed: true
   });
 
+  let running = require("internal").getExternalSpawned();
+  let i = 0;
+  for (i = 0; i < running.length; i++) {
+    let status = require("internal").statusExternal(running[i].pid, false);
+    if (status.status === "TERMINATED") {
+      print("process exited without us joining it (marking crashy): " + JSON.stringify(running[i]) + JSON.stringify(status));
+    }
+    else {
+      print("Killing remaining process & marking crashy: " + JSON.stringify(running[i]));
+      print(require("internal").killExternal(running[i].pid, abortSignal));
+    }
+    res.crashed = true;
+  };
+
   // whether or not there was an error
   try {
     fs.write(testOutputDirectory + '/UNITTEST_RESULT_EXECUTIVE_SUMMARY.json', String(res.status), true);
@@ -313,12 +327,6 @@ function main (argv) {
   // creates yaml like dump at the end
   UnitTest.unitTestPrettyPrintResults(res, testOutputDirectory, options);
 
-  let running = require("internal").getExternalSpawned();
-  let i = 0;
-  for (i = 0; i < running.length; i++) {
-    print("Killing remaining process: " + JSON.stringify(running[i]));
-    print(require("internal").killExternal(running[i].pid, abortSignal));
-  };
   return res.status && running.length === 0;
 }
 

js/client/modules/@arangodb/crash-utils.js

@@ -278,9 +278,7 @@ function analyzeCrash (binary, instanceInfo, options, checkStr) {
       instanceInfo.exitStatus['gdbHint'] = "coredump unavailable";
       return;
     }
-    if (instanceInfo.monitor.pid !== null) {
+    pu.stopProcdump(options, instanceInfo);
-      instanceInfo.monitor = statusExternal(instanceInfo.monitor.pid, true);
-    }
     hint = analyzeCoreDumpWindows(instanceInfo);
   } else if (platform === 'darwin') {
     // fs.copyFile(binary, storeArangodPath);

js/client/modules/@arangodb/process-utils.js

@@ -498,6 +498,14 @@ function runProcdump (options, instanceInfo, rootDir, pid) {
   }
 }
 
+function stopProcdump (options, instanceInfo) {
+  if (instanceInfo.hasOwnProperty('monitor') &&
+      instanceInfo.monitor.pid !== null) {
+    print("wating for procdump to exit");
+    statusExternal(instanceInfo.monitor.pid, true);
+    instanceInfo.monitor.pid = null;
+  }
+}
 // //////////////////////////////////////////////////////////////////////////////
 // / @brief executes a command and waits for result
 // //////////////////////////////////////////////////////////////////////////////
@@ -562,6 +570,7 @@ function executeAndWait (cmd, args, options, valgrindTest, rootDir, circumventCo
     runProcdump(options, instanceInfo, rootDir, res.pid);
     Object.assign(instanceInfo.exitStatus, 
                   statusExternal(res.pid, true));
+    stopProcdump(options, instanceInfo);
   } else {
     res = executeExternalAndWait(cmd, args);
     instanceInfo.pid = res.pid;
@@ -1105,11 +1114,13 @@ function shutdownInstance (instanceInfo, options, forceTerminate) {
           print("shutdownInstance: Marking crashy - " + JSON.stringify(arangod));
           serverCrashedLocal = true;
         }
+        stopProcdump(options, arangod);
       } else {
         if (arangod.role !== 'agent') {
           nonAgenciesCount --;
         }
         print('Server "' + arangod.role + '" shutdown: Success: pid', arangod.pid);
+        stopProcdump(options, arangod);
         return false;
       }
     });
@@ -1570,6 +1581,7 @@ exports.findFreePort = findFreePort;
 
 exports.executeArangod = executeArangod;
 exports.executeAndWait = executeAndWait;
+exports.stopProcdump = stopProcdump;
 
 exports.createBaseConfig = createBaseConfigBuilder;
 exports.run = {

lib/Basics/process-utils.cpp

@@ -98,15 +98,8 @@ ExternalId::ExternalId():
 ExternalProcess::ExternalProcess():
   _numberArguments(0),
   _arguments(nullptr),
-#ifndef _WIN32
+#ifdef _WIN32
-  _pid(0),
-  _readPipe(-1),
-  _writePipe(-1),
-#else
-  _pid(0),
   _process(nullptr),
-  _readPipe(INVALID_HANDLE_VALUE),
-  _writePipe(INVALID_HANDLE_VALUE),
 #endif
   _status(TRI_EXT_NOT_STARTED),
   _exitStatus(0) {}
@@ -1087,7 +1080,7 @@ ExternalProcessStatus TRI_CheckExternalProcess(ExternalId pid,
             wantGetExitCode = true;
             break;
           case WAIT_TIMEOUT:
-            // success - everything went well.
+            // success - process is up and running.
             external->_exitStatus = 0;
             break;
           case WAIT_FAILED:
@@ -1117,6 +1110,8 @@ ExternalProcessStatus TRI_CheckExternalProcess(ExternalId pid,
               std::string("exit status could not be determined for pid ") +
               arangodb::basics::StringUtils::itoa(
                   static_cast<int64_t>(external->_pid));
+          external->_exitStatus = -1;
+          external->_status = TRI_EXT_NOT_STARTED;
         } else {
           if (exitCode == STILL_ACTIVE) {
             external->_exitStatus = 0;

lib/Basics/process-utils.h

@@ -100,20 +100,13 @@ struct ExternalId {
 /// @brief external process description
 ////////////////////////////////////////////////////////////////////////////////
 
-struct ExternalProcess {
+struct ExternalProcess : public ExternalId {
   std::string _executable;
   size_t _numberArguments;
   char** _arguments;
 
-#ifndef _WIN32
+#ifdef _WIN32
-  TRI_pid_t _pid;
-  int _readPipe;
-  int _writePipe;
-#else
-  DWORD _pid;
   HANDLE _process;
-  HANDLE _readPipe;
-  HANDLE _writePipe;
 #endif
 
   TRI_external_status_e _status;

lib/V8/v8-utils.cpp

@@ -3522,10 +3522,10 @@ static void JS_HMAC(v8::FunctionCallbackInfo<v8::Value> const& args) {
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief Convert programm stati to V8
 ////////////////////////////////////////////////////////////////////////////////
-static char const* convertProcessStatusToString(ExternalProcessStatus external) {
+static char const* convertProcessStatusToString(TRI_external_status_e processStatus) {
    char const* status = "UNKNOWN";
 
-  switch (external._status) {
+  switch (processStatus) {
     case TRI_EXT_NOT_STARTED:
       status = "NOT-STARTED";
       break;
@@ -3556,7 +3556,7 @@ static char const* convertProcessStatusToString(ExternalProcessStatus external)
 
 static void convertPipeStatus(v8::FunctionCallbackInfo<v8::Value> const& args,
                               v8::Handle<v8::Object> &result,
-                              ExternalId &external) {
+                              ExternalId const& external) {
   TRI_V8_TRY_CATCH_BEGIN(isolate);
 
   result->Set(TRI_V8_ASCII_STRING(isolate, "pid"),
@@ -3593,14 +3593,14 @@ static void convertPipeStatus(v8::FunctionCallbackInfo<v8::Value> const& args,
 
 static void convertStatusToV8(v8::FunctionCallbackInfo<v8::Value> const& args,
                                v8::Handle<v8::Object> &result,
-                               ExternalProcessStatus &external_status,
+                               ExternalProcessStatus const& external_status,
-                               ExternalId &external) {
+                               ExternalId const& external) {
   TRI_V8_TRY_CATCH_BEGIN(isolate);
 
   convertPipeStatus(args, result, external);
 
   result->Set(TRI_V8_ASCII_STRING(isolate, "status"),
-              TRI_V8_ASCII_STRING(isolate, convertProcessStatusToString(external_status)));
+              TRI_V8_ASCII_STRING(isolate, convertProcessStatusToString(external_status._status)));
 
   if (external_status._status == TRI_EXT_TERMINATED) {
     result->Set(TRI_V8_ASCII_STRING(isolate, "exit"),
@@ -3618,6 +3618,36 @@ static void convertStatusToV8(v8::FunctionCallbackInfo<v8::Value> const& args,
   TRI_V8_TRY_CATCH_END;
 }
 
+static void convertProcessInfoToV8(v8::FunctionCallbackInfo<v8::Value> const& args,
+                                   v8::Handle<v8::Object> &result,
+                                   ExternalProcess const& external_process) {
+  TRI_V8_TRY_CATCH_BEGIN(isolate);
+
+  convertPipeStatus(args, result, external_process);
+
+  result->Set(TRI_V8_ASCII_STRING(isolate, "status"),
+              TRI_V8_ASCII_STRING(isolate, convertProcessStatusToString(external_process._status)));
+
+  if (external_process._status == TRI_EXT_TERMINATED) {
+    result->Set(TRI_V8_ASCII_STRING(isolate, "exit"),
+                v8::Integer::New(isolate, static_cast<int32_t>(
+                                              external_process._exitStatus)));
+  } else if (external_process._status == TRI_EXT_ABORTED) {
+    result->Set(TRI_V8_ASCII_STRING(isolate, "signal"),
+                v8::Integer::New(isolate, static_cast<int32_t>(
+                                              external_process._exitStatus)));
+  }
+  result->Set(TRI_V8_ASCII_STRING(isolate, "executable"),
+              TRI_V8_STD_STRING(isolate, external_process._executable));
+
+  v8::Handle<v8::Array> arguments =
+      v8::Array::New(isolate, static_cast<int>(external_process._numberArguments));
+  for (size_t i = 0; i < external_process._numberArguments; i++) {
+    arguments->Set(i, TRI_V8_ASCII_STRING(isolate, external_process._arguments[i]));
+  }
+  result->Set(TRI_V8_ASCII_STRING(isolate, "arguments"), arguments);
+  TRI_V8_TRY_CATCH_END;
+}
 ////////////////////////////////////////////////////////////////////////////////
 /// @brief lists all running external processes
 ////////////////////////////////////////////////////////////////////////////////
@@ -3638,10 +3668,7 @@ static void JS_GetExternalSpawned(
   uint32_t i = 0;
   for (auto const& process : ExternalProcesses) {
     v8::Handle<v8::Object> oneProcess = v8::Object::New(isolate);
-    ExternalId external;
+    convertProcessInfoToV8(args, oneProcess, *process);
-    external._pid = process->_pid;
-    auto external_status = TRI_CheckExternalProcess(external, false);
-    convertStatusToV8(args, oneProcess, external_status, external);
     spawnedProcesses->Set(i, oneProcess);
     i++;
   }
@@ -3750,7 +3777,7 @@ static void JS_StatusExternal(v8::FunctionCallbackInfo<v8::Value> const& args) {
   v8::Handle<v8::Object> result = v8::Object::New(isolate);
 
   result->Set(TRI_V8_ASCII_STRING(isolate, "status"),
-              TRI_V8_STRING(isolate, convertProcessStatusToString(external)));
+              TRI_V8_STRING(isolate, convertProcessStatusToString(external._status)));
 
   if (external._status == TRI_EXT_TERMINATED) {
     result->Set(