Proposed fix for deadlock on shutdown (#1673)

* Proposed fix for deadlock on shutdown

* Terminate C style comment.

* Added needed libs for applicable tests and updated the logic for when allowing/disallowing connections.

* Need to load additional trick libs for applicable tests for Linux.

---------

Co-authored-by: Hong Chen <hong.chen-1@nasa.gov>
This commit is contained in:
jmpenn 2024-03-28 15:15:19 -05:00 committed by GitHub
parent 72e82e1566
commit c4e60d9f9a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
12 changed files with 65 additions and 22 deletions

View File

@ -66,6 +66,8 @@ namespace Trick {
*/
int freeze_init() ;
void shutdownConnections();
/**
@brief Shutdown the variable server
@return always 0

View File

@ -58,6 +58,8 @@ namespace Trick {
virtual void dump( std::ostream & oss = std::cout ) ;
void shutdownConnections();
protected:
void initializeMulticast();
@ -82,9 +84,13 @@ namespace Trick {
/* Multicast broadcaster */
MulticastGroup * _multicast; /**< trick_io(**) trick_units(--) */
bool allowConnections; /**< trick_io(**) trick_units(--) */
unsigned int pendingConnections; /**< trick_io(**) trick_units(--) */
pthread_mutex_t connectionMutex; /**< trick_io(**) trick_units(--) */
pthread_cond_t noPendingConnections_cv; /**< trick_io(**) trick_units(--) */
} ;
}
#endif

View File

@ -9,7 +9,7 @@ include $(dir $(lastword $(MAKEFILE_LIST)))../../../../share/trick/makefiles/Mak
# Flags passed to the preprocessor.
TRICK_CPPFLAGS += -I$(GTEST_HOME)/include -I$(TRICK_HOME)/include -g -Wall -Wextra ${TRICK_SYSTEM_CXXFLAGS} ${TRICK_TEST_FLAGS}
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick -ltrick_units -ltrick_mm -ltrick_pyip -ltrick_connection_handlers -ltrick_comm
LIBS = -L${GTEST_HOME}/lib64 -L${GTEST_HOME}/lib -lgtest -lgtest_main
@ -41,7 +41,7 @@ GetTimeOfDayClock_test.o : GetTimeOfDayClock_test.cpp
$(TRICK_CXX) $(TRICK_CPPFLAGS) -c $<
GetTimeOfDayClock_test : ${GETTIMEOFDAY_CLOCK_OBJECTS}
$(TRICK_CXX) $(TRICK_SYSTEM_LDFLAGS) $(TRICK_CPPFLAGS) -o $@ $^ ${LIBS}
$(TRICK_CXX) $(TRICK_SYSTEM_LDFLAGS) $(TRICK_CPPFLAGS) -o $@ $^ $(TRICK_LIBS) ${LIBS}
exec_get_rt_nap_stub.o : exec_get_rt_nap_stub.cpp
$(TRICK_CXX) $(TRICK_CPPFLAGS) -c $<

View File

@ -17,7 +17,7 @@ TRICK_CXXFLAGS += -I$(GTEST_HOME)/include -I$(TRICK_HOME)/include -g -Wall -Wext
MM_OBJECTS = $(TRICK_HOME)/trick_source/sim_services/MemoryManager/object_${TRICK_HOST_CPU}/extract_bitfield.o \
$(TRICK_HOME)/trick_source/sim_services/MemoryManager/object_${TRICK_HOST_CPU}/extract_unsigned_bitfield.o
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick -ltrick_mm -ltrick_units -ltrick
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick -ltrick_mm -ltrick_units -ltrick -ltrick_pyip -ltrick_connection_handlers -ltrick_comm -ltrick_mm -ltrick_units -ltrick
TRICK_EXEC_LINK_LIBS += -L${GTEST_HOME}/lib64 -L${GTEST_HOME}/lib -lgtest -lgtest_main -lpthread

View File

@ -19,6 +19,8 @@
#include "trick/message_type.h"
#include "trick/release.h"
#include "trick/SysThread.hh"
#include "trick/VariableServer.hh"
extern Trick::VariableServer * the_vs ;
/**
@design
@ -62,6 +64,9 @@ int Trick::Executive::shutdown() {
}
}
/* Stop new connections to the Variable Server. */
the_vs->shutdownConnections();
try {
/* Call the shutdown jobs. */

View File

@ -12,7 +12,7 @@ COVERAGE_FLAGS += -fprofile-arcs -ftest-coverage -O0
# Flags passed to the preprocessor.
TRICK_CPPFLAGS += -I$(GTEST_HOME)/include -I$(TRICK_HOME)/include -g -Wall -Wextra ${TRICK_SYSTEM_CXXFLAGS} ${TRICK_TEST_FLAGS}
TRICK_LIBS = -L ${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick
TRICK_LIBS = -L ${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick -ltrick_pyip -ltrick_connection_handlers -ltrick_comm
TRICK_EXEC_LINK_LIBS += -L${GTEST_HOME}/lib64 -L${GTEST_HOME}/lib -lgtest -lgtest_main
TRICK_SYSTEM_LDFLAGS += ${COVERAGE_FLAGS}

View File

@ -10,7 +10,7 @@ include $(dir $(lastword $(MAKEFILE_LIST)))../../../../share/trick/makefiles/Mak
# Flags passed to the preprocessor.
TRICK_CPPFLAGS += -I$(GTEST_HOME)/include -I$(TRICK_HOME)/include -I${TRICK_HOME}/trick_source -I${TRICK_HOME}/include/trick/compat -g -Wall -Wextra -DUSE_ER7_UTILS_INTEGRATORS=1 -DTEST ${TRICK_SYSTEM_CXXFLAGS} ${TRICK_TEST_FLAGS}
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick -ltrick_mm -ltrick_units -ltrick -ltrick_mm
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick -ltrick_mm -ltrick_units -ltrick_mm -ltrick_pyip -ltrick_connection_handlers -ltrick_comm -ltrick_mm -ltrick_units -ltrick
TRICK_EXEC_LINK_LIBS += -L${GTEST_HOME}/lib64 -L${GTEST_HOME}/lib -lgtest -lgtest_main
# All tests produced by this Makefile. Remember to add new tests you

View File

@ -14,7 +14,7 @@ TRICK_SYSTEM_LDFLAGS += ${COVERAGE_FLAGS}
# Flags passed to the preprocessor.
TRICK_CPPFLAGS += -I$(GTEST_HOME)/include -I$(TRICK_HOME)/include -g -Wall -Wextra -Wno-sign-compare ${COVERAGE_FLAGS} ${TRICK_SYSTEM_CXXFLAGS} ${TRICK_TEST_FLAGS}
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick -ltrick_mm -ltrick_units -ltrick
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick -ltrick_mm -ltrick_units -ltrick -ltrick_connection_handlers -ltrick_pyip -ltrick -ltrick_comm
TRICK_EXEC_LINK_LIBS += -L${GTEST_HOME}/lib64 -L${GTEST_HOME}/lib -lgtest -lgtest_main -lpthread
# ==================================================================================

View File

@ -9,7 +9,7 @@ include $(dir $(lastword $(MAKEFILE_LIST)))../../../../share/trick/makefiles/Mak
# Flags passed to the preprocessor.
TRICK_CPPFLAGS += -I$(GTEST_HOME)/include -I$(TRICK_HOME)/include -g -Wall -Wextra ${TRICK_SYSTEM_CXXFLAGS} ${TRICK_TEST_FLAGS}
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick -ltrick_mm -ltrick_units -ltrick
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick_mm -ltrick_units -ltrick -ltrick_mm -ltrick_units -ltrick -ltrick_pyip -ltrick_connection_handlers -ltrick_comm -ltrick
TRICK_EXEC_LINK_LIBS += -L${GTEST_HOME}/lib64 -L${GTEST_HOME}/lib -lgtest -lgtest_main
# All tests produced by this Makefile. Remember to add new tests you

View File

@ -9,7 +9,7 @@ include $(dir $(lastword $(MAKEFILE_LIST)))../../../../share/trick/makefiles/Mak
# Flags passed to the preprocessor.
TRICK_CPPFLAGS += -I$(GTEST_HOME)/include -I$(TRICK_HOME)/include -g -Wall -Wextra ${TRICK_SYSTEM_CXXFLAGS} ${TRICK_TEST_FLAGS}
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick -ltrick_units -ltrick_mm
TRICK_LIBS = -L${TRICK_LIB_DIR} -ltrick -ltrick_units -ltrick_mm -ltrick_pyip -ltrick_connection_handlers -ltrick_comm
TRICK_EXEC_LINK_LIBS += -L${GTEST_HOME}/lib64 -L${GTEST_HOME}/lib -lgtest -lgtest_main -lpthread
# All tests produced by this Makefile. Remember to add new tests you

View File

@ -19,6 +19,10 @@ Trick::VariableServer::~VariableServer() {
the_vs = NULL;
}
void Trick::VariableServer::shutdownConnections() {
listen_thread.shutdownConnections();
}
std::ostream& Trick::operator<< (std::ostream& s, Trick::VariableServer& vs) {
std::map < pthread_t , VariableServerSessionThread * >::iterator it ;

View File

@ -34,6 +34,11 @@ Trick::VariableServerListenThread::VariableServerListenThread(TCPClientListener
_listener = new TCPClientListener;
}
allowConnections = true;
pendingConnections = 0;
pthread_mutex_init( &connectionMutex, NULL);
pthread_cond_init( &noPendingConnections_cv, NULL);
cancellable = false;
}
@ -142,7 +147,7 @@ void * Trick::VariableServerListenThread::thread_body() {
} else {
user_name = strdup(passp->pw_name) ;
}
_listener->setBlockMode(true);
if ( _broadcast ) {
@ -160,18 +165,29 @@ void * Trick::VariableServerListenThread::thread_body() {
if (_listener->checkForNewConnections()) {
// Create a new thread to service this connection
VariableServerSessionThread * vst = new Trick::VariableServerSessionThread() ;
vst->set_connection(_listener->setUpNewConnection());
vst->copy_cpus(get_cpus()) ;
vst->create_thread() ;
ConnectionStatus status = vst->wait_for_accept() ;
if ( allowConnections) {
pthread_mutex_lock(&connectionMutex);
pendingConnections ++;
if (status == CONNECTION_FAIL) {
// If the connection failed, the thread will exit.
// Make sure it joins fully before deleting the vst object
vst->join_thread();
delete vst;
VariableServerSessionThread * vst = new Trick::VariableServerSessionThread() ;
vst->set_connection(_listener->setUpNewConnection());
vst->copy_cpus(get_cpus()) ;
vst->create_thread() ;
ConnectionStatus status = vst->wait_for_accept() ;
if (status == CONNECTION_FAIL) {
// If the connection failed, the thread will exit.
// Make sure it joins fully before deleting the vst object
vst->join_thread();
delete vst;
}
pendingConnections --;
if ( pendingConnections == 0 ) {
pthread_cond_signal( &noPendingConnections_cv );
}
pthread_mutex_unlock(&connectionMutex);
}
} else if ( _broadcast ) {
// Otherwise, broadcast on the multicast channel if enabled
char buf1[1024];
@ -192,6 +208,17 @@ void * Trick::VariableServerListenThread::thread_body() {
return NULL ;
}
void Trick::VariableServerListenThread::shutdownConnections() {
pthread_mutex_lock(&connectionMutex);
allowConnections = false;
// if ANY connections are pending, then wait here until were notified that NO connections are pending.
if (pendingConnections > 0) {
allowConnections = true;
pthread_cond_wait( &noPendingConnections_cv, &connectionMutex);
}
pthread_mutex_unlock( &connectionMutex );
}
#include <fcntl.h>
int Trick::VariableServerListenThread::restart() {
@ -210,7 +237,7 @@ int Trick::VariableServerListenThread::restart() {
_listener->disconnect();
ret = _listener->initialize(_requested_source_address, _requested_port);
if (ret != 0) {
message_publish(MSG_ERROR, "ERROR: Could not establish listen port %d for Variable Server. Aborting.\n", _requested_port);
return (-1);
@ -250,4 +277,3 @@ void Trick::VariableServerListenThread::dump( std::ostream & oss ) {
oss << " broadcast = " << _broadcast << std::endl ;
Trick::ThreadBase::dump(oss) ;
}