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 <[email protected]>
nasa · Mar 28, 2024 · c4e60d9 · c4e60d9
1 parent 72e82e1
commit c4e60d9
Show file tree

Hide file tree

Showing 12 changed files with 66 additions and 23 deletions.
diff --git a/include/trick/VariableServer.hh b/include/trick/VariableServer.hh
@@ -66,6 +66,8 @@ namespace Trick {
             */
             int freeze_init() ;
 
+            void shutdownConnections();
+
             /**
              @brief Shutdown the variable server
              @return always 0

diff --git a/include/trick/VariableServerListenThread.hh b/include/trick/VariableServerListenThread.hh
@@ -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
-
diff --git a/trick_source/sim_services/Clock/test/Makefile b/trick_source/sim_services/Clock/test/Makefile
@@ -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 $<
diff --git a/trick_source/sim_services/CommandLineArguments/test/Makefile b/trick_source/sim_services/CommandLineArguments/test/Makefile
@@ -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
 
 

diff --git a/trick_source/sim_services/Executive/Executive_shutdown.cpp b/trick_source/sim_services/Executive/Executive_shutdown.cpp
@@ -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. */

diff --git a/trick_source/sim_services/Executive/test/Makefile b/trick_source/sim_services/Executive/test/Makefile
@@ -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}
 

diff --git a/trick_source/sim_services/Integrator/test/Makefile b/trick_source/sim_services/Integrator/test/Makefile
@@ -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

diff --git a/trick_source/sim_services/MemoryManager/test/Makefile b/trick_source/sim_services/MemoryManager/test/Makefile
@@ -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
 
 # ==================================================================================

diff --git a/trick_source/sim_services/ScheduledJobQueue/test/Makefile b/trick_source/sim_services/ScheduledJobQueue/test/Makefile
@@ -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

diff --git a/trick_source/sim_services/Timer/test/Makefile b/trick_source/sim_services/Timer/test/Makefile
@@ -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

diff --git a/trick_source/sim_services/VariableServer/VariableServer.cpp b/trick_source/sim_services/VariableServer/VariableServer.cpp
@@ -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 ;
 

diff --git a/trick_source/sim_services/VariableServer/VariableServerListenThread.cpp b/trick_source/sim_services/VariableServer/VariableServerListenThread.cpp
@@ -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 (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;
+            if ( allowConnections) {
+                pthread_mutex_lock(&connectionMutex);
+                pendingConnections ++;
+
+                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 we’re 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) ;
 }
-