Skip to content

Commit

Permalink
wait for procdump to exit too. (#7731)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@dothebart @mchacki
dothebart authored and mchacki committed Dec 12, 2018
1 parent a19f90c commit 7acb3b3
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 38 deletions.
20 changes: 14 additions & 6 deletions UnitTests/unittest.js
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
}

Expand Down
4 changes: 1 addition & 3 deletions js/client/modules/@arangodb/crash-utils.js
View file
Expand Up @@ -278,9 +278,7 @@ function analyzeCrash (binary, instanceInfo, options, checkStr) {
instanceInfo.exitStatus['gdbHint'] = "coredump unavailable";
return;
}
if (instanceInfo.monitor.pid !== null) {
instanceInfo.monitor = statusExternal(instanceInfo.monitor.pid, true);
}
pu.stopProcdump(options, instanceInfo);
hint = analyzeCoreDumpWindows(instanceInfo);
} else if (platform === 'darwin') {
// fs.copyFile(binary, storeArangodPath);
Expand Down
12 changes: 12 additions & 0 deletions js/client/modules/@arangodb/process-utils.js
View file
Expand Up @@ -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
// //////////////////////////////////////////////////////////////////////////////
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
}
});
Expand Down Expand Up @@ -1570,6 +1581,7 @@ exports.findFreePort = findFreePort;

exports.executeArangod = executeArangod;
exports.executeAndWait = executeAndWait;
exports.stopProcdump = stopProcdump;

exports.createBaseConfig = createBaseConfigBuilder;
exports.run = {
Expand Down
13 changes: 4 additions & 9 deletions lib/Basics/process-utils.cpp
View file
Expand Up @@ -98,15 +98,8 @@ ExternalId::ExternalId():
ExternalProcess::ExternalProcess():
_numberArguments(0),
_arguments(nullptr),
#ifndef _WIN32
_pid(0),
_readPipe(-1),
_writePipe(-1),
#else
_pid(0),
#ifdef _WIN32
_process(nullptr),
_readPipe(INVALID_HANDLE_VALUE),
_writePipe(INVALID_HANDLE_VALUE),
#endif
_status(TRI_EXT_NOT_STARTED),
_exitStatus(0) {}
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down
11 changes: 2 additions & 9 deletions lib/Basics/process-utils.h
View file
Expand Up @@ -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
TRI_pid_t _pid;
int _readPipe;
int _writePipe;
#else
DWORD _pid;
#ifdef _WIN32
HANDLE _process;
HANDLE _readPipe;
HANDLE _writePipe;
#endif

TRI_external_status_e _status;
Expand Down
49 changes: 38 additions & 11 deletions lib/V8/v8-utils.cpp
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -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"),
Expand Down Expand Up @@ -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,
ExternalId &external) {
ExternalProcessStatus const& external_status,
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"),
Expand All @@ -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
////////////////////////////////////////////////////////////////////////////////
Expand All @@ -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;
external._pid = process->_pid;
auto external_status = TRI_CheckExternalProcess(external, false);
convertStatusToV8(args, oneProcess, external_status, external);
convertProcessInfoToV8(args, oneProcess, *process);
spawnedProcesses->Set(i, oneProcess);
i++;
}
Expand Down Expand Up @@ -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(
Expand Down

0 comments on commit 7acb3b3

Please sign in to comment.