From 9c7becb4ffa655c78d05e58a8d71f7f72117875b Mon Sep 17 00:00:00 2001 From: Jacqueline Deans Date: Wed, 8 Feb 2023 17:58:59 -0600 Subject: [PATCH] Handle thread shutdown gracefully and test (#1448) --------- Co-authored-by: Dan Jordan --- include/trick/DataRecordDispatcher.hh | 4 +- include/trick/MessageTCDevice.hh | 4 +- include/trick/MessageThreadedCout.hh | 4 +- include/trick/SysThread.hh | 56 ++++++++++++++ include/trick/ThreadBase.hh | 6 ++ include/trick/VariableServer.hh | 2 +- include/trick/VariableServerListenThread.hh | 4 +- include/trick/VariableServerThread.hh | 4 +- test/SIM_earlyterm/RUN_test/input.py | 1 + test/SIM_earlyterm/S_define | 31 ++++++++ test_sims.yml | 7 ++ .../DataRecord/DataRecordDispatcher.cpp | 2 +- .../Executive/Executive_shutdown.cpp | 3 + .../sim_services/Message/MessageTCDevice.cpp | 5 +- .../Message/MessageThreadedCout.cpp | 4 +- trick_source/sim_services/ThreadBase/Makefile | 1 + .../sim_services/ThreadBase/SysThread.cpp | 77 +++++++++++++++++++ .../sim_services/ThreadBase/ThreadBase.cpp | 7 ++ .../VariableServerListenThread.cpp | 2 +- .../VariableServer/VariableServerThread.cpp | 2 +- .../trick_utils/var_binary_parser/Makefile | 5 +- 21 files changed, 212 insertions(+), 19 deletions(-) create mode 100644 include/trick/SysThread.hh create mode 100644 test/SIM_earlyterm/RUN_test/input.py create mode 100644 test/SIM_earlyterm/S_define create mode 100644 trick_source/sim_services/ThreadBase/SysThread.cpp diff --git a/include/trick/DataRecordDispatcher.hh b/include/trick/DataRecordDispatcher.hh index 55d58932..f1509da2 100644 --- a/include/trick/DataRecordDispatcher.hh +++ b/include/trick/DataRecordDispatcher.hh @@ -13,7 +13,7 @@ PROGRAMMERS: #include "trick/Scheduler.hh" #include "trick/DataRecordGroup.hh" -#include "trick/ThreadBase.hh" +#include "trick/SysThread.hh" namespace Trick { @@ -32,7 +32,7 @@ namespace Trick { bool cancelled; } ; - class DRDWriterThread : public Trick::ThreadBase { + class DRDWriterThread : public Trick::SysThread { public: DRDWriterThread(Trick::DRDMutexes & in_mutexes, std::vector & in_groups) ; diff --git a/include/trick/MessageTCDevice.hh b/include/trick/MessageTCDevice.hh index 8d2656e1..c117903b 100644 --- a/include/trick/MessageTCDevice.hh +++ b/include/trick/MessageTCDevice.hh @@ -7,14 +7,14 @@ #include #include "trick/MessageSubscriber.hh" -#include "trick/ThreadBase.hh" +#include "trick/SysThread.hh" #include "trick/tc.h" namespace Trick { class MessageTCDevice ; - class MessageTCDeviceListenThread : public Trick::ThreadBase { + class MessageTCDeviceListenThread : public Trick::SysThread { public: MessageTCDeviceListenThread(MessageTCDevice * in_mtcd) ; diff --git a/include/trick/MessageThreadedCout.hh b/include/trick/MessageThreadedCout.hh index 740fc9d1..dfef3f76 100644 --- a/include/trick/MessageThreadedCout.hh +++ b/include/trick/MessageThreadedCout.hh @@ -7,7 +7,7 @@ #define MESSAGETHREADEDCOUT_HH #include -#include "trick/ThreadBase.hh" +#include "trick/SysThread.hh" #include "trick/MessageSubscriber.hh" namespace Trick { @@ -16,7 +16,7 @@ namespace Trick { * This MessageThreadedCout is a class that inherits from MessageSubscriber. * It defines a type of MessageSubscriber with its received message sending to the standard output stream. */ - class MessageThreadedCout : public MessageSubscriber , public Trick::ThreadBase { + class MessageThreadedCout : public MessageSubscriber , public Trick::SysThread { public: diff --git a/include/trick/SysThread.hh b/include/trick/SysThread.hh new file mode 100644 index 00000000..a565b8c5 --- /dev/null +++ b/include/trick/SysThread.hh @@ -0,0 +1,56 @@ +/* + PURPOSE: + (Trick Sys Threads implementation) +*/ + +#ifndef SYSTHREAD_HH +#define SYSTHREAD_HH + +#include +#include +#include +#include +#include +#if __linux +#include +#endif +#include +#include +#include "trick/ThreadBase.hh" + + +namespace Trick { + + /** + * The purpose of this class is to ensure safe shutdown for Trick system threads, since user threads are handled separately in + * the Trick::Threads and Executive classes. + * + * This class was implemented as a solution to issue https://github.com/nasa/trick/issues/1445 + * + * @author Jackie Deans + * + * + **/ + class SysThread : public Trick::ThreadBase { + public: + SysThread(std::string in_name, bool self_deleting = false); + ~SysThread(); + + static int ensureAllShutdown(); + + private: + // Had to use Construct On First Use here to avoid the static initialziation fiasco + static pthread_mutex_t& list_mutex(); + static pthread_cond_t& list_empty_cv(); + + static std::vector & all_sys_threads(); + + static bool shutdown_finished; + + bool self_deleting; + } ; + +} + +#endif + diff --git a/include/trick/ThreadBase.hh b/include/trick/ThreadBase.hh index 085b0b76..41ec3273 100644 --- a/include/trick/ThreadBase.hh +++ b/include/trick/ThreadBase.hh @@ -130,6 +130,12 @@ namespace Trick { */ virtual int cancel_thread() ; + /** + * Cancels thread. + * @return always 0 + */ + virtual int join_thread() ; + /** * The thread body. * @return always 0 diff --git a/include/trick/VariableServer.hh b/include/trick/VariableServer.hh index c65951b0..10cf4ee2 100644 --- a/include/trick/VariableServer.hh +++ b/include/trick/VariableServer.hh @@ -18,7 +18,7 @@ #include "trick/variable_server_sync_types.h" #include "trick/VariableServerThread.hh" #include "trick/VariableServerListenThread.hh" -#include "trick/ThreadBase.hh" +#include "trick/SysThread.hh" namespace Trick { diff --git a/include/trick/VariableServerListenThread.hh b/include/trick/VariableServerListenThread.hh index e773d3cb..4b7af4e6 100644 --- a/include/trick/VariableServerListenThread.hh +++ b/include/trick/VariableServerListenThread.hh @@ -9,7 +9,7 @@ #include #include #include "trick/tc.h" -#include "trick/ThreadBase.hh" +#include "trick/SysThread.hh" namespace Trick { @@ -17,7 +17,7 @@ namespace Trick { This class runs the variable server listen loop. @author Alex Lin */ - class VariableServerListenThread : public Trick::ThreadBase { + class VariableServerListenThread : public Trick::SysThread { public: VariableServerListenThread() ; diff --git a/include/trick/VariableServerThread.hh b/include/trick/VariableServerThread.hh index c92ef72c..fed81f59 100644 --- a/include/trick/VariableServerThread.hh +++ b/include/trick/VariableServerThread.hh @@ -11,7 +11,7 @@ #include #include #include "trick/tc.h" -#include "trick/ThreadBase.hh" +#include "trick/SysThread.hh" #include "trick/VariableServerReference.hh" #include "trick/variable_server_sync_types.h" #include "trick/variable_server_message_types.h" @@ -25,7 +25,7 @@ namespace Trick { This class provides variable server command processing on a separate thread for each client. @author Alex Lin */ - class VariableServerThread : public Trick::ThreadBase { + class VariableServerThread : public Trick::SysThread { public: enum ConnectionType { TCP, UDP, MCAST } ; diff --git a/test/SIM_earlyterm/RUN_test/input.py b/test/SIM_earlyterm/RUN_test/input.py new file mode 100644 index 00000000..242edf3f --- /dev/null +++ b/test/SIM_earlyterm/RUN_test/input.py @@ -0,0 +1 @@ +#Nothing diff --git a/test/SIM_earlyterm/S_define b/test/SIM_earlyterm/S_define new file mode 100644 index 00000000..79bcc0a7 --- /dev/null +++ b/test/SIM_earlyterm/S_define @@ -0,0 +1,31 @@ +/***************************************************************************** +PURPOSE: Provide test of simulation early termination. Ensures threads + come down appropriately for unit-test-like cases +PROGRAMMERS: + (((Dan Jordan) (NASA) (Jan 2023) (Deal with it))) +*****************************************************************************/ +#include "sim_objects/default_trick_sys.sm" +##include "trick/exec_proto.h" + +class EarlyTerminationSimObject : public Trick::SimObject +{ + public: + double x; + + EarlyTerminationSimObject() + : + x(0) + { + ("initialization") early_term(); + }; + + void early_term() { + std::string message = "Terminating with exit code 0"; + exec_terminate_with_return(0, "S_define", 24, message.c_str()); + } + private: + EarlyTerminationSimObject( const EarlyTerminationSimObject&); + EarlyTerminationSimObject & operator= ( const EarlyTerminationSimObject&); + +}; +EarlyTerminationSimObject test; diff --git a/test_sims.yml b/test_sims.yml index 68df7f32..db89be36 100644 --- a/test_sims.yml +++ b/test_sims.yml @@ -189,6 +189,13 @@ SIM_sun: runs: RUN_test/unit_test.py: returns: 0 +SIM_earlyterm: + path: test/SIM_earlyterm + build_command: "trick-CP -t" + binary: "T_main_{cpu}_test.exe" + runs: + RUN_test/input.py: + returns: 0 # Special cases diff --git a/trick_source/sim_services/DataRecord/DataRecordDispatcher.cpp b/trick_source/sim_services/DataRecord/DataRecordDispatcher.cpp index df5533d7..19911b21 100644 --- a/trick_source/sim_services/DataRecord/DataRecordDispatcher.cpp +++ b/trick_source/sim_services/DataRecord/DataRecordDispatcher.cpp @@ -33,7 +33,7 @@ Trick::DRDMutexes::DRDMutexes() { } Trick::DRDWriterThread::DRDWriterThread(DRDMutexes & in_mutexes, std::vector & in_groups) : - ThreadBase("DR_Writer"), + SysThread("DR_Writer"), drd_mutexes(in_mutexes) , groups(in_groups) {} diff --git a/trick_source/sim_services/Executive/Executive_shutdown.cpp b/trick_source/sim_services/Executive/Executive_shutdown.cpp index e453ed80..0d6dbdc9 100644 --- a/trick_source/sim_services/Executive/Executive_shutdown.cpp +++ b/trick_source/sim_services/Executive/Executive_shutdown.cpp @@ -17,6 +17,7 @@ #include "trick/message_proto.h" #include "trick/message_type.h" #include "trick/release.h" +#include "trick/SysThread.hh" /** @design @@ -75,6 +76,8 @@ int Trick::Executive::shutdown() { except_message += std::string(" then exception Message: ") + ex.message ; } + Trick::SysThread::ensureAllShutdown(); + getrusage(RUSAGE_SELF, &cpu_usage_buf); cpu_time = ((double) cpu_usage_buf.ru_utime.tv_sec) + ((double) cpu_usage_buf.ru_utime.tv_usec / 1000000.0); diff --git a/trick_source/sim_services/Message/MessageTCDevice.cpp b/trick_source/sim_services/Message/MessageTCDevice.cpp index 6849f69a..e0c0fde3 100644 --- a/trick_source/sim_services/Message/MessageTCDevice.cpp +++ b/trick_source/sim_services/Message/MessageTCDevice.cpp @@ -8,7 +8,7 @@ #include "trick/tc_proto.h" Trick::MessageTCDeviceListenThread::MessageTCDeviceListenThread(MessageTCDevice * in_mtcd) : - Trick::ThreadBase("MessageListen"), + Trick::SysThread("MessageListen"), mtcd(in_mtcd) , listen_dev() { /* And a TCDevice for message server @e listen_device is configured. */ @@ -25,6 +25,7 @@ Trick::MessageTCDeviceListenThread::MessageTCDeviceListenThread(MessageTCDevice Trick::MessageTCDeviceListenThread::~MessageTCDeviceListenThread() { free(listen_dev.error_handler) ; + listen_dev.error_handler = NULL; if ( listen_dev.hostname ) { free((char*)listen_dev.hostname) ; } @@ -81,7 +82,7 @@ void * Trick::MessageTCDeviceListenThread::thread_body() { if (status == TC_SUCCESS) { mtcd->add_connection(new_connection) ; - } + } } } diff --git a/trick_source/sim_services/Message/MessageThreadedCout.cpp b/trick_source/sim_services/Message/MessageThreadedCout.cpp index c2ca6348..99fe0c3e 100644 --- a/trick_source/sim_services/Message/MessageThreadedCout.cpp +++ b/trick_source/sim_services/Message/MessageThreadedCout.cpp @@ -10,10 +10,10 @@ Trick::MessageThreadedCout::MessageThreadedCout() : max_buffer_size(4000) , print_immediate(false) , copy_ptr(NULL), - write_ptr(NULL) { + write_ptr(NULL), + SysThread("threadedcout") { /** By default, this subscriber is enabled when it is created. */ Trick::MessageSubscriber::name = "threadedcout" ; - Trick::ThreadBase::name = "threadedcout" ; color_code.reserve(6) ; StringNode * temp = new StringNode(max_buffer_size) ; write_ptr = copy_ptr = temp ; diff --git a/trick_source/sim_services/ThreadBase/Makefile b/trick_source/sim_services/ThreadBase/Makefile index 6f722048..44838eb2 100644 --- a/trick_source/sim_services/ThreadBase/Makefile +++ b/trick_source/sim_services/ThreadBase/Makefile @@ -1,3 +1,4 @@ include $(dir $(lastword $(MAKEFILE_LIST)))../../../share/trick/makefiles/Makefile.common include ${TRICK_HOME}/share/trick/makefiles/Makefile.tricklib -include Makefile_deps +TRICK_CXXFLAGS += -std=c++11 \ No newline at end of file diff --git a/trick_source/sim_services/ThreadBase/SysThread.cpp b/trick_source/sim_services/ThreadBase/SysThread.cpp new file mode 100644 index 00000000..cfb98f6f --- /dev/null +++ b/trick_source/sim_services/ThreadBase/SysThread.cpp @@ -0,0 +1,77 @@ + +#include +#include +#include +#if __linux +#include +#include +#include +#endif +#include +#include + +#include "trick/SysThread.hh" + +bool Trick::SysThread::shutdown_finished = false; + +// Construct On First Use to avoid the Static Initialization Fiasco +pthread_mutex_t& Trick::SysThread::list_mutex() { + static pthread_mutex_t list_mutex = PTHREAD_MUTEX_INITIALIZER; + return list_mutex; +} + +pthread_cond_t& Trick::SysThread::list_empty_cv() { + static pthread_cond_t list_empty_cv = PTHREAD_COND_INITIALIZER; + return list_empty_cv; +} + +std::vector& Trick::SysThread::all_sys_threads() { + static std::vector all_sys_threads; + return all_sys_threads; +} + +Trick::SysThread::SysThread(std::string in_name, bool sd) : self_deleting(sd), ThreadBase(in_name) { + pthread_mutex_lock(&(list_mutex())); + all_sys_threads().push_back(this); + pthread_mutex_unlock(&(list_mutex())); +} + + +Trick::SysThread::~SysThread() { + pthread_mutex_lock(&(list_mutex())); + if (!shutdown_finished) { + all_sys_threads().erase(std::remove(all_sys_threads().begin(), all_sys_threads().end(), this), all_sys_threads().end()); + if (all_sys_threads().size() == 0) { + pthread_cond_signal(&(list_empty_cv())); + } + } + pthread_mutex_unlock(&(list_mutex())); +} + +int Trick::SysThread::ensureAllShutdown() { + pthread_mutex_lock(&(list_mutex())); + + for (SysThread * thread : all_sys_threads()) { + thread->cancel_thread(); + } + + auto it = all_sys_threads().begin(); + while (it != all_sys_threads().end()){ + SysThread * thread = *it; + if (!(thread->self_deleting)) { + thread->join_thread(); + it = all_sys_threads().erase(it); + } else { + ++it; + } + } + + while (all_sys_threads().size() != 0) { + pthread_cond_wait(&(list_empty_cv()), &(list_mutex())); + } + + shutdown_finished = true; + pthread_mutex_unlock(&(list_mutex())); + + return 0; +} \ No newline at end of file diff --git a/trick_source/sim_services/ThreadBase/ThreadBase.cpp b/trick_source/sim_services/ThreadBase/ThreadBase.cpp index f924ab51..11af01e7 100644 --- a/trick_source/sim_services/ThreadBase/ThreadBase.cpp +++ b/trick_source/sim_services/ThreadBase/ThreadBase.cpp @@ -300,6 +300,13 @@ int Trick::ThreadBase::cancel_thread() { return(0) ; } +int Trick::ThreadBase::join_thread() { + if ( pthread_id != 0 ) { + pthread_join(pthread_id, NULL) ; + } + return(0) ; +} + void * Trick::ThreadBase::thread_helper( void * context ) { sigset_t sigs; diff --git a/trick_source/sim_services/VariableServer/VariableServerListenThread.cpp b/trick_source/sim_services/VariableServer/VariableServerListenThread.cpp index 00f6e868..17408e38 100644 --- a/trick_source/sim_services/VariableServer/VariableServerListenThread.cpp +++ b/trick_source/sim_services/VariableServer/VariableServerListenThread.cpp @@ -11,7 +11,7 @@ #include "trick/message_type.h" Trick::VariableServerListenThread::VariableServerListenThread() : - Trick::ThreadBase("VarServListen"), + Trick::SysThread("VarServListen"), port(0), user_port_requested(false), broadcast(true), diff --git a/trick_source/sim_services/VariableServer/VariableServerThread.cpp b/trick_source/sim_services/VariableServer/VariableServerThread.cpp index 4cc33293..45b75747 100644 --- a/trick_source/sim_services/VariableServer/VariableServerThread.cpp +++ b/trick_source/sim_services/VariableServer/VariableServerThread.cpp @@ -8,7 +8,7 @@ Trick::VariableServer * Trick::VariableServerThread::vs = NULL ; Trick::VariableServerThread::VariableServerThread(TCDevice * in_listen_dev) : - Trick::ThreadBase("VarServer") , + Trick::SysThread("VarServer", true) , listen_dev(in_listen_dev) { debug = 0 ; diff --git a/trick_source/trick_utils/var_binary_parser/Makefile b/trick_source/trick_utils/var_binary_parser/Makefile index b816c264..5c956098 100644 --- a/trick_source/trick_utils/var_binary_parser/Makefile +++ b/trick_source/trick_utils/var_binary_parser/Makefile @@ -42,8 +42,11 @@ ${LIBDIR}: ${TRICK_LIB}: ${LIBDIR}/${LIBNAME} cp ${LIBDIR}/${LIBNAME} $(TRICK_LIB) +real_clean: clean + clean: - ${RM} -r ${OBJDIR} + ${RM} -rf ${OBJDIR} + ${RM} -rf ${LIBDIR} ${MAKE} -C test clean spotless: