From 6fd80a31c513668c82412861b1e983b7a6bda364 Mon Sep 17 00:00:00 2001 From: Phanes Date: Sat, 2 Dec 2017 04:22:09 -0500 Subject: [PATCH] potential fix for subprocess execution (exit code was wonky) --- CMakeLists.txt | 4 +- conf/units/all_test.units | 2 +- src/loaders/Plan.cpp | 19 ++++-- src/loaders/Suite.cpp | 10 +-- src/loaders/Task.cpp | 140 +++++++++++++++++--------------------- src/sproc/Sproc.cpp | 40 ++++++++--- 6 files changed, 115 insertions(+), 100 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index d54ac0b..d187857 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,7 +1,7 @@ cmake_minimum_required(VERSION 3.5) project(ftests) - set(CMAKE_CXX_STANDARD 11) - +set(CMAKE_CXX_FLAGS_DEBUG "${CMAKE_CXX_FLAGS_DEBUG} -std=c++1z -O0 -DDEBUG=1") set(SOURCE_FILES examplar.cpp src/loaders/loaders.cpp src/loaders/loaders.h src/json/jsoncpp.cpp src/loaders/JSON_Loader.cpp src/loaders/JSON_Loader.h src/loaders/helpers.cpp src/loaders/helpers.h src/loaders/Suite.cpp src/loaders/Suite.h src/loaders/Plan.cpp src/loaders/Plan.h src/loaders/Conf.cpp src/loaders/Conf.h src/loaders/Unit.cpp src/loaders/Unit.h src/loaders/Task.cpp src/loaders/Task.h src/sproc/Sproc.cpp src/sproc/Sproc.h) + add_executable(ftests ${SOURCE_FILES}) \ No newline at end of file diff --git a/conf/units/all_test.units b/conf/units/all_test.units index 92a4170..ccc4e4d 100644 --- a/conf/units/all_test.units +++ b/conf/units/all_test.units @@ -6,7 +6,7 @@ "rectifier": "/usr/bin/true", "active": true, "required": true, - "rectify": false + "rectify": true }, { "name": "independent test 2", diff --git a/src/loaders/Plan.cpp b/src/loaders/Plan.cpp index 3786389..1af8937 100644 --- a/src/loaders/Plan.cpp +++ b/src/loaders/Plan.cpp @@ -114,12 +114,17 @@ void Plan::load_definitions( Suite unit_definitions, bool verbose ) void Plan::execute( bool verbose ) { // for each task in this plan - for ( int i = 0; i < this->tasks.size(); i++ ) - { - if ( verbose ) { - std::cout << "Executing task \"" << this->tasks[i].get_name() << "\"." << std::endl; - } - this->tasks[i].execute( verbose ); - } +// for ( int i = 0; i < this->tasks.size(); i++ ) +// { +// if ( verbose ) { +// std::cout << "Executing task \"" << this->tasks[i].get_name() << "\"." << std::endl; + std::cout << "Executing task \"" << this->tasks[0].get_name() << "\"." << std::endl; +// } +// this->tasks[i].execute( verbose ); + this->tasks[0].execute( verbose ); + + // for testing a logic issue in Task.execute(), remove when done +// throw Plan_InvalidTaskIndex(); +// } } diff --git a/src/loaders/Suite.cpp b/src/loaders/Suite.cpp index be6adc3..cc23803 100644 --- a/src/loaders/Suite.cpp +++ b/src/loaders/Suite.cpp @@ -48,10 +48,12 @@ void Suite::load_units_file( std::string filename, bool verbose ) { // assemble the unit from json_root using the built-in value operator tmp_U.load_root( this->json_root[ index ] ); - // append to this->units - this->units.push_back( tmp_U ); - if ( verbose ) { - std::cout << "Added unit \"" << tmp_U.get_name() << "\" to Suite." << std::endl; + if ( tmp_U.get_active() ) { + // append to this->units + this->units.push_back( tmp_U ); + if ( verbose ) { + std::cout << "Added unit \"" << tmp_U.get_name() << "\" to Suite." << std::endl; + } } } } diff --git a/src/loaders/Task.cpp b/src/loaders/Task.cpp index 7157864..686a4c2 100644 --- a/src/loaders/Task.cpp +++ b/src/loaders/Task.cpp @@ -4,28 +4,33 @@ #include "../sproc/Sproc.h" /// Task_InvalidDataStructure - Exception thrown when a Task is defined with invalid JSON. -class Task_InvalidDataStructure: public std::runtime_error { public: +class Task_InvalidDataStructure: public std::runtime_error { +public: Task_InvalidDataStructure(): std::runtime_error("Task: Attempted to access a member of a Task that is not set.") {} }; /// Task_InvalidDataStructure - Exception thrown when a Task is defined with invalid JSON. -class Task_NotReady: public std::runtime_error { public: +class Task_NotReady: public std::runtime_error { +public: Task_NotReady(): std::runtime_error("Task: Attempted to access a unit of a Task that is not defined.") {} }; /// Task_RequiredButFailedTask - Exception thrown when a Task is failed but required, and rectification also failed. -class Task_RequiredButFailedTask: public std::runtime_error { public: - Task_RequiredButFailedTask(): std::runtime_error("Task: Attempted to access a unit of a Task that failed, but was required, and the corresponding rectification target also failed..") {} +class Task_RequiredButFailedTask: public std::runtime_error { +public: + Task_RequiredButFailedTask(): std::runtime_error("Task: Attempted to execute a Task that failed and was required.") {} }; /// Task_RequiredButFailedTask - Exception thrown when a Task is failed but required, and rectification also failed but returned with a zero exit code (dont try to fool the check). -class Task_RequiredButRectifierDoesNotHeal: public std::runtime_error { public: +class Task_RequiredButRectifierDoesNotHeal: public std::runtime_error { +public: Task_RequiredButRectifierDoesNotHeal(): std::runtime_error("Task: The rectification script was executed and reported success, but did not actually heal the faulty condition of the Task target.") {} }; /// Task::Task() - Constructor for the Task class. The Task is the building block of a Plan indicating of which Unit to /// execute, and its dependencies on other units to have already been completed successfully. -Task::Task() { +Task::Task() +{ // it hasn't executed yet. this->complete = false; @@ -39,10 +44,10 @@ Task::Task() { /// \param verbose - Whether to print verbose information to STDOUT. void Task::load_root(Json::Value loader_root, bool verbose ) { - if ( loader_root.isMember("name") ) - { + if ( loader_root.isMember("name") ) { this->name = loader_root.get("name", "?").asString(); - } else { + } + else { throw Task_InvalidDataStructure(); } @@ -50,14 +55,11 @@ void Task::load_root(Json::Value loader_root, bool verbose ) Json::Value des_dep_root = loader_root.get("dependencies", 0); // iterate through each member of that obj - for ( int i = 0; i < des_dep_root.size(); i++ ) - { + for ( int i = 0; i < des_dep_root.size(); i++ ) { // add each string to dependencies - if ( des_dep_root[i].asString() != "" ) - { + if ( des_dep_root[i].asString() != "" ) { this->dependencies.push_back( des_dep_root[i].asString() ); - if ( verbose ) - { + if ( verbose ) { std::cout << "Added dependency \"" << des_dep_root[i].asString() << "\" to task \"" << this->get_name() << "\"." << std::endl; } } @@ -77,20 +79,21 @@ std::string Task::get_name() void Task::load_definition( Unit selected_unit, bool verbose ) { this->definition = selected_unit; - if ( verbose ) - { + if ( verbose ) { std::cout << "Loaded definition \"" << selected_unit.get_name() << "\" for task \"" << this->get_name() << "\"." << std::endl; } this->defined = true; } /// Task::is_complete - Indicator if the task executed successfully. -bool Task::is_complete() { +bool Task::is_complete() +{ return this->complete; } /// Task::has_definition - Indicator if the task has attached its definition from a Suite. -bool Task::has_definition() { +bool Task::has_definition() +{ return this->defined; } @@ -103,7 +106,9 @@ void Task::execute( bool verbose ) // PREWORK // throw if unit not coupled to all necessary values since Task is stateful (stateful is okay) - if (! this->has_definition() ) { throw Task_NotReady(); } + if (! this->has_definition() ) { + throw Task_NotReady(); + } // get the name std::string task_name = this->definition.get_name(); @@ -114,8 +119,7 @@ void Task::execute( bool verbose ) std::string target_command = this->definition.get_target(); // if we're in verbose mode, do some verbose things - if ( verbose ) - { + if ( verbose ) { std::cout << "\tUsing unit \"" << task_name << "\"." << std::endl; std::cout << "\tExecuting target \"" << target_command << "\"." << std::endl; } @@ -124,23 +128,18 @@ void Task::execute( bool verbose ) int return_code = Sproc::execute( target_command ); // d[0] check exit code of target - if (return_code == 0) - { + if (return_code == 0) { // Zero d[0] return from target execution, good to return - if ( verbose ) - { + if ( verbose ) { std::cout << "\tTarget " << task_name << " succeeded." << std::endl; } // next - return; - } else { // Non-Zero d[0] from initial target execution, get to d[1] std::cout << "\tTarget \"" << task_name << "\" failed with exit code " << return_code << "." << std::endl; // check if rectify pattern is enabled d[1] - if ( this->definition.get_rectify() ) - { + if ( this->definition.get_rectify() ) { // yes d[1] std::cout << "\tRectification pattern is enabled for \"" << task_name << "\"." << std::endl; // execute RECTIFIER @@ -149,64 +148,49 @@ void Task::execute( bool verbose ) int rectifier_error = Sproc::execute( rectifier_command ); // d[3] check exit code of rectifier - if ( rectifier_error ) - { + if (rectifier_error) { //d[3] non-zero std::cout << "\tRectification of \"" << task_name << "\" failed with exit code " << rectifier_error << "." << std::endl; // d[2] check if REQUIRED - if ( this->definition.get_required() ) - { + if ( this->definition.get_required() ) { // d[2] yes // halt/exception throw Task_RequiredButFailedTask(); - } else { - // d[2] no - // next - return; } - } else { - // d[3] zero - - // execute target - std::cout << "\tRe-Executing target \"" << this->definition.get_target() << "\"." << std::endl; - int retry_code = Sproc::execute( target_command ); - - // d[4] exit code of target retry - if (retry_code == 0) { - // d[4] zero - return; - } else { - // d[4] non-zero - // d[5] required check - if ( this->definition.get_required() ) - { - // d[5] yes - throw Task_RequiredButRectifierDoesNotHeal(); - } else { - // d[5] no - // next - return; - } - } - } - } else { - // no d[1] - std::cout << "\tRectification is not enabled for \"" << task_name << "\"." << std::endl; - // required d[2] - if ( this->definition.get_required() ) - { - // d[2] yes - // This is executing..... - std::cout << "\tThis task is required to continue the plan." << std::endl; - // but these are NOT executing????? - throw Task_RequiredButFailedTask(); - return; - } else { // d[2] no - std::cout << "\tThis task is not required to continue the plan." << std::endl; - return; + // next } + // d[3] zero + + // execute target + std::cout << "\tRe-Executing target \"" << this->definition.get_target() << "\"." << std::endl; + int retry_code = Sproc::execute( target_command ); + + // d[4] exit code of target retry + if (retry_code == 0) { + // d[4] zero + } + // d[4] non-zero + // d[5] required check + if ( this->definition.get_required() ) { + // d[5] yes + std::cout << "\tTask \"" << task_name << "\" is required but rectification did not heal." << std::endl; + throw Task_RequiredButRectifierDoesNotHeal(); + } + // d[5] no + // next } + // no d[1] + std::cout << "\tRectification is not enabled for \"" << task_name << "\"." << std::endl; + // required d[2] + if ( this->definition.get_required() ) { + // d[2] yes + // This is executing..... + std::cout << "\tThis task is required to continue the plan." << std::endl; + // but these are NOT executing????? + throw Task_RequiredButFailedTask(); + } // d[2] no + std::cout << "\tThis task is not required to continue the plan." << std::endl; } } \ No newline at end of file diff --git a/src/sproc/Sproc.cpp b/src/sproc/Sproc.cpp index 882721a..ca7092c 100644 --- a/src/sproc/Sproc.cpp +++ b/src/sproc/Sproc.cpp @@ -1,6 +1,7 @@ #include "Sproc.h" #include #include +#include /// Sproc::execute /// @@ -13,14 +14,17 @@ int Sproc::execute(std::string input) { int stdin_pipe[2]; int stderr_pipe[2]; int stdout_pipe[2]; - int child_pid; + pid_t child_pid, w; char nChar; - int child_exit_code; + int child_exit_code = -666; + + // experimental + int status; if ( pipe(stdin_pipe) < 0 ) { perror("allocating pipe for child input redirect"); - return -1; + exit(-1); } if ( pipe(stdout_pipe) < 0 ) @@ -28,14 +32,14 @@ int Sproc::execute(std::string input) { close(stdin_pipe[PIPE_READ]); close(stdin_pipe[PIPE_WRITE]); perror("allocating pipe for child output redirect"); - return -1; + exit(-1); } if ( pipe(stderr_pipe) < 0 ) { close(stderr_pipe[PIPE_READ]); close(stderr_pipe[PIPE_WRITE]); perror("allocating pipe for error redirect"); - return -1; + exit(-1); } @@ -74,13 +78,31 @@ int Sproc::execute(std::string input) { // replace this with any exec* function find easier to use ("man exec") child_exit_code = system( input.c_str() ); - // if we get here at all, an error occurred, but we are in the child // process, so just exit - return WEXITSTATUS(child_exit_code); + exit(WEXITSTATUS(child_exit_code)); } else if (child_pid > 0) { // parent continues here + do { + w = waitpid(child_pid, &child_exit_code, WUNTRACED | WCONTINUED); + if (w == -1) { + perror("waitpid"); + exit(EXIT_FAILURE); + } + + if (WIFEXITED(child_exit_code)) { + ;; + //printf("exited, status=%d\n", WEXITSTATUS(child_exit_code)); + } else if (WIFSIGNALED(child_exit_code)) { + printf("killed by signal %d\n", WTERMSIG(child_exit_code)); + } else if (WIFSTOPPED(child_exit_code)) { + printf("stopped by signal %d\n", WSTOPSIG(child_exit_code)); + } else if (WIFCONTINUED(child_exit_code)) { + printf("continued\n"); + } + } while (!WIFEXITED(child_exit_code) && !WIFSIGNALED(child_exit_code)); + // close unused file descriptors, these are for child only close(stdin_pipe[PIPE_READ]); close(stdout_pipe[PIPE_WRITE]); @@ -100,6 +122,7 @@ int Sproc::execute(std::string input) { close(stderr_pipe[PIPE_READ]); } else { // failed to create child + std::cout << "Failed to create child." << std::endl; close(stdin_pipe[PIPE_READ]); close(stdin_pipe[PIPE_WRITE]); @@ -109,6 +132,7 @@ int Sproc::execute(std::string input) { close(stderr_pipe[PIPE_READ]); close(stderr_pipe[PIPE_WRITE]); } + child_exit_code = WEXITSTATUS( child_exit_code ); - return WEXITSTATUS(child_exit_code); + return child_exit_code; } \ No newline at end of file