30672691247

Committed 31 Jul 2026 11:22PM UTC coverage: 56.743% (+0.07%) from 56.671%

Build # 30672691247

Build Type

Pull #2173

github

Committed by

web-flow

Commit Message

Merge fd95a7456 into 746816931

Pull Request Pull Request #2173: Fix GIL shutdown hang.

Coverage Stats

46 of 55 new or added lines in 3 files covered. (83.64%)

14869 of 26204 relevant lines covered (56.74%)

463203.35 hits per line

Source File
Press 'n' to go to next uncovered line, 'b' for previous

89.66

/trick_source/sim_services/VariableServer/VariableServer_shutdown.cpp

#include "trick/VariableServer.hh"

#include "trick/message_proto.h"
#include "trick/message_type.h"

#include <pthread.h>
#include <unistd.h>
#include <vector>

namespace
{

    // How long to give session threads to finish their current command and exit before
    // giving up on them. Commands are normally sub-millisecond, so this only comes into
    // play when a client command is blocked inside a model call.
    const unsigned int SESSION_SHUTDOWN_TIMEOUT_USEC = 5000000;
    const unsigned int SESSION_SHUTDOWN_POLL_USEC    = 1000;

}

int Trick::VariableServer::shutdown() {
    // Shut down all listen threads. These only ever block in accept() and never enter the
    // Python interpreter, so an asynchronous cancel is safe for them.
    listen_thread.cancel_thread() ;
    for (auto& listen_it : additional_listen_threads) {
        listen_it.second->cancel_thread();
    }

    // Shut down all session threads.
    //
    // A session thread may be part-way through executing a client command through the
    // input processor, which holds the Python GIL (IPPython::parse()). pthread_cancel()ing
    // it there -- which is what cancel_thread() does -- can destroy the thread while it
    // still owns the GIL, orphaning the GIL so that ip.shutdown() hangs forever trying to
    // reacquire it.
    //
    // Ask each thread to stop instead and let it finish its current command and release
    // the GIL on its own. This keeps the existing vs.shutdown() -> ip.shutdown() job
    // ordering meaningful: by the time this job returns, well-behaved sessions are gone
    // and the GIL is free.
    std::vector<VariableServerSessionThread*> sessions;
    pthread_mutex_lock(&map_mutex) ;
    for (auto& thread : var_server_threads)
    {
        thread.second->request_shutdown();
        sessions.push_back(thread.second);
    }
    pthread_mutex_unlock(&map_mutex) ;

    // Wait for the threads to drop out of the map, which they do from exit_var_thread() on
    // their way out.
    //
    // This wait is deliberately bounded. A client command blocked inside a model call
    // never returns, so waiting indefinitely (or calling join_thread() straight away)
    // would just relocate the hang from ip.shutdown() into this job, where no
    // later safety net can catch it.
    unsigned int waited = 0;
    while (waited < SESSION_SHUTDOWN_TIMEOUT_USEC)
    {
        pthread_mutex_lock(&map_mutex);
        bool all_stopped = var_server_threads.empty();
        pthread_mutex_unlock(&map_mutex);

        if (all_stopped)
        {
            break;
        }

        usleep(SESSION_SHUTDOWN_POLL_USEC);
        waited += SESSION_SHUTDOWN_POLL_USEC;
    }

    // Reap the threads that stopped and abandon the ones that did not.
    //
    // Joining is done outside map_mutex on purpose: an exiting thread runs
    // exit_var_thread(), which takes map_mutex in delete_session()/delete_vst(), so
    // holding it across a join here would deadlock.
    for (auto* thread : sessions)
    {
        pthread_mutex_lock(&map_mutex);
        bool still_running = (var_server_threads.count(thread->get_pthread_id()) > 0);
        pthread_mutex_unlock(&map_mutex);

        if (still_running)
        {
            message_publish(MSG_WARNING,
                            "Trick::VariableServer::shutdown() giving up on a variable server session thread that did "
                            "not stop when asked; it is most likely blocked inside a client command. Abandoning it so "
                            "that shutdown can continue rather than hanging.\n");
            thread->abandon_thread();
        }
        else
        {
            thread->join_thread();
        }
    }

    return 0 ;
}

1	#include "trick/VariableServer.hh"
2
3	#include "trick/message_proto.h"
4	#include "trick/message_type.h"
5
6	#include <pthread.h>
7	#include <unistd.h>
8	#include <vector>
9
10	namespace
11	{
12
13	// How long to give session threads to finish their current command and exit before
14	// giving up on them. Commands are normally sub-millisecond, so this only comes into
15	// play when a client command is blocked inside a model call.
16	const unsigned int SESSION_SHUTDOWN_TIMEOUT_USEC = 5000000;
17	const unsigned int SESSION_SHUTDOWN_POLL_USEC = 1000;
18
19	}
20
21	int Trick::VariableServer::shutdown() {	156✔
22	// Shut down all listen threads. These only ever block in accept() and never enter the
23	// Python interpreter, so an asynchronous cancel is safe for them.
24	listen_thread.cancel_thread() ;	156✔
25	for (auto& listen_it : additional_listen_threads) {	156✔
26	listen_it.second->cancel_thread();	×
27	}
28
29	// Shut down all session threads.
30	//
31	// A session thread may be part-way through executing a client command through the
32	// input processor, which holds the Python GIL (IPPython::parse()). pthread_cancel()ing
33	// it there -- which is what cancel_thread() does -- can destroy the thread while it
34	// still owns the GIL, orphaning the GIL so that ip.shutdown() hangs forever trying to
35	// reacquire it.
36	//
37	// Ask each thread to stop instead and let it finish its current command and release
38	// the GIL on its own. This keeps the existing vs.shutdown() -> ip.shutdown() job
39	// ordering meaningful: by the time this job returns, well-behaved sessions are gone
40	// and the GIL is free.
41	std::vector<VariableServerSessionThread*> sessions;	156✔
42	pthread_mutex_lock(&map_mutex) ;	156✔
43	for (auto& thread : var_server_threads)	160✔
44	{
45	thread.second->request_shutdown();	4✔
46	sessions.push_back(thread.second);	4✔
47	}
48	pthread_mutex_unlock(&map_mutex) ;	156✔
49
50	// Wait for the threads to drop out of the map, which they do from exit_var_thread() on
51	// their way out.
52	//
53	// This wait is deliberately bounded. A client command blocked inside a model call
54	// never returns, so waiting indefinitely (or calling join_thread() straight away)
55	// would just relocate the hang from ip.shutdown() into this job, where no
56	// later safety net can catch it.
57	unsigned int waited = 0;	156✔
58	while (waited < SESSION_SHUTDOWN_TIMEOUT_USEC)	344✔
59	{
60	pthread_mutex_lock(&map_mutex);	344✔
61	bool all_stopped = var_server_threads.empty();	344✔
62	pthread_mutex_unlock(&map_mutex);	344✔
63
64	if (all_stopped)	344✔
65	{
66	break;	156✔
67	}
68
69	usleep(SESSION_SHUTDOWN_POLL_USEC);	188✔
70	waited += SESSION_SHUTDOWN_POLL_USEC;	188✔
71	}
72
73	// Reap the threads that stopped and abandon the ones that did not.
74	//
75	// Joining is done outside map_mutex on purpose: an exiting thread runs
76	// exit_var_thread(), which takes map_mutex in delete_session()/delete_vst(), so
77	// holding it across a join here would deadlock.
78	for (auto* thread : sessions)	160✔
79	{
80	pthread_mutex_lock(&map_mutex);	4✔
81	bool still_running = (var_server_threads.count(thread->get_pthread_id()) > 0);	4✔
82	pthread_mutex_unlock(&map_mutex);	4✔
83
84	if (still_running)	4✔
85	{
NEW 86	message_publish(MSG_WARNING,	×
87	"Trick::VariableServer::shutdown() giving up on a variable server session thread that did "
88	"not stop when asked; it is most likely blocked inside a client command. Abandoning it so "
89	"that shutdown can continue rather than hanging.\n");
NEW 90	thread->abandon_thread();	×
91	}
92	else
93	{
94	thread->join_thread();	4✔
95	}
96	}
97
98	return 0 ;	156✔
99	}	156✔

nasa / trick / 30672691247

Source File Press 'n' to go to next uncovered line, 'b' for previous

Source File
Press 'n' to go to next uncovered line, 'b' for previous