Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid calling PyEval_RestoreThread when python is finalizing #9

Closed
wants to merge 1 commit into from

Conversation

Junchao-Mellanox
Copy link
Owner

@Junchao-Mellanox Junchao-Mellanox commented May 15, 2024

Why I did this?

sonic_swss_common provides a way to allow multi-threading while python is calling c++ code:

#ifdef SWIGPYTHON
%exception {
    try
    {
        class PyThreadStateGuard
        {
            PyThreadState *m_save;
        public:
            PyThreadStateGuard()
            {
                m_save = PyEval_SaveThread();
            }
            ~PyThreadStateGuard()
            {
                PyEval_RestoreThread(m_save);
            }
        } thread_state_guard;

        $action
    }
    SWIG_CATCH_STDEXCEPT // catch std::exception derivatives
    catch (...)
    {
        SWIG_exception(SWIG_UnknownError, "unknown exception");
    }
}

This code calls PyEval_RestoreThread in a destructor which would potentially cause unexpected thread termination:

(gdb) bt
#0  0x00007f883f152ce1 in raise () from /lib/x86_64-linux-gnu/libc.so.6
#1  0x00007f883f13c537 in abort () from /lib/x86_64-linux-gnu/libc.so.6
#2  0x00007f883d68e7ec in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#3  0x00007f883d699966 in ?? () from /lib/x86_64-linux-gnu/libstdc++.so.6
#4  0x00007f883d6999d1 in std::terminate() () from /lib/x86_64-linux-gnu/libstdc++.so.6
#5  0x00007f883d6993cc in __gxx_personality_v0 () from /lib/x86_64-linux-gnu/libstdc++.so.6
#6  0x00007f883e1028a4 in ?? () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#7  0x00007f883e102f4e in _Unwind_ForcedUnwind () from /lib/x86_64-linux-gnu/libgcc_s.so.1
#8  0x00007f883f49cc30 in __pthread_unwind () from /lib/x86_64-linux-gnu/libpthread.so.0
#9  0x00007f883f49418c in pthread_exit () from /lib/x86_64-linux-gnu/libpthread.so.0
#10 0x0000000000645df5 in PyThread_exit_thread ()
#11 0x00000000004262ae in ?? ()
#12 0x00000000005327b2 in PyEval_RestoreThread ()
#13 0x00007f883df41a6e in ?? () from /usr/lib/python3/dist-packages/swsscommon/_swsscommon.so
#14 0x000000000053f350 in ?? ()
#15 0x000000000051d89b in _PyObject_MakeTpCall ()
#16 0x00000000005175ba in _PyEval_EvalFrameDefault ()
#17 0x00000000005106ed in ?? ()
#18 0x0000000000528d21 in _PyFunction_Vectorcall ()
#19 0x0000000000512192 in _PyEval_EvalFrameDefault ()
#20 0x0000000000528b63 in _PyFunction_Vectorcall ()
#21 0x0000000000512192 in _PyEval_EvalFrameDefault ()
#22 0x0000000000528b63 in _PyFunction_Vectorcall ()
#23 0x0000000000512192 in _PyEval_EvalFrameDefault ()
#24 0x0000000000528b63 in _PyFunction_Vectorcall ()
#25 0x000000000053bdb1 in ?? ()
#26 0x00000000006430b6 in ?? ()
#27 0x000000000063b164 in ?? ()
#28 0x00007f883f492ea7 in start_thread () from /lib/x86_64-linux-gnu/libpthread.so.0
#29 0x00007f883f215a6f in clone () from /lib/x86_64-linux-gnu/libc.so.6

This issue is found on python3 and there is a statement in python3 official doc about PyEval_RestoreThread https://docs.python.org/3/c-api/init.html:

Note Calling this function from a thread when the runtime is finalizing will terminate the thread, even if the thread was not created by Python. You can use _Py_IsFinalizing() or [sys.is_finalizing()](https://docs.python.org/3/library/sys.html#sys.is_finalizing) to check if the interpreter is in process of being finalized before calling this function to avoid unwanted termination.

According to the official doc, _Py_IsFinalizing() should be checked before calling PyEval_RestoreThread. The PR is to fix this issue.

@Junchao-Mellanox Junchao-Mellanox force-pushed the master-crash-releasing-gil branch from 7725a5b to 9ae771f Compare May 16, 2024 06:42
@Junchao-Mellanox Junchao-Mellanox force-pushed the master-crash-releasing-gil branch from 9ae771f to 1d0d993 Compare May 16, 2024 07:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant