Race condition in subinterpreters during subinterpreter creation on Windows debug build #100711

Activity

1. Fidget-Spinner

   added

   type-bugAn unexpected behavior, bug, or errorAn unexpected behavior, bug, or error

   on Jan 3, 2023
2. Fidget-Spinner commented on Jan 3, 2023

   

   Fidget-Spinner

   on Jan 3, 2023

   MemberAuthor

   More actions

   cc @ericsnowcurrently
3. Fidget-Spinner

   changed the title [-]Race condition in subinterpreters during subinterpreter creation on Windows[/-] [+]Race condition in subinterpreters during subinterpreter creation on Windows debug build[/+] on Jan 3, 2023
4. AlexWaygood

   added

   topic-subinterpreters

   on Jan 3, 2023
5. ezio-melotti

   added this to Subinterpreterson Jan 3, 2023
6. ericsnowcurrently commented on Jan 4, 2023

   

   ericsnowcurrently

   on Jan 4, 2023

   Member

   More actions

   I'll take a look tomorrow. Thanks for finding this!

   👍React with 👍1Reacted by Ken Jin
7. ericsnowcurrently commented on Jan 4, 2023

   

   ericsnowcurrently

   on Jan 4, 2023

   Member

   More actions

   What's the best way to reproduce the problem?
8. Fidget-Spinner commented on Jan 4, 2023

   

   Fidget-Spinner

   on Jan 4, 2023

   MemberAuthor

   More actions

   Sorry, I have not found a reliable/minimal reproducer. The only thing I found was that running
   .\PCbuild\amd64\python_d.exe -m test test__xxsubinterpreters -F on Windows
   eventually triggers it (sometimes it takes 100 test iterations). If you don't get a crash by then, try rerunning it.

   👍React with 👍1Reacted by Eric Snow
9. ericsnowcurrently commented on Jan 4, 2023

   

   ericsnowcurrently

   on Jan 4, 2023

   Member

   More actions

   Is the race in the main branch? Any thoughts on why you noticed it while working on gh-100492?
10. Fidget-Spinner commented on Jan 4, 2023

    

    Fidget-Spinner

    on Jan 4, 2023

    MemberAuthor

    More actions

    Yep it's on main. I'm not sure why but it requires less iterations to trigger on that PR. It requires many more iterations on main.
11. ericsnowcurrently commented on Jan 4, 2023

    

    ericsnowcurrently

    on Jan 4, 2023

    Member

    More actions

    I took a quick look at ceval_gil.c and may have a sense of what's going wrong. FWIW, I expect that the race has been around at least since the "new GIL" was added in 2009, if not longer, and that _xxsubinterpreters has only brought it to our attention (rather than causing the problem).

    The main thing I noticed (in ceval_gil.c) is that both drop_gil() and take_gil() take a PyThreadState argument for the thread that is taking/releasing the GIL, AKA "thread A". It is possible for that thread state to be destroyed (directly, or indirectly when its interpreter is destroyed) in another thread, AKA "thread B", at any point the GIL is no longer held by thread A.

    If the given thread state (thread A) were used at any such point within drop_gil() or take_gil() then I'd expect a segfault. Here are places where that is possible:

    • drop_gil()
      • before GIL-protecting mutex aquired
        • set _PyRuntimeState.ceval.gil.last_holder to thread A
      • after GIL-protecting mutex released
        • read tstate->interp->ceval.gil_drop_request
        • compared tstate to _PyRuntimeState.ceval.gil.last_holder
        • RESET_GIL_DROP_REQUEST() called with tstate->interp
    • take_gil()
      • before GIL-protecting mutex acquired
        • read tstate->interp (used after GIL mutex acquired)
        • read tstate->interp->ceval (used after GIL mutex acquired)
      • while waiting for GIL
        • (during interpreter/runtime fini) RESET_GIL_DROP_REQUEST() called with tstate->interp
        • SET_GIL_DROP_REQUEST() called with tstate->interp
      • after GIL acquired (but thread A might have been destroyed while waiting)
        • set _PyRuntimeState.ceval.gil.last_holder to thread A
        • (during interpreter/runtime fini) drop_gil() called with tstate
        • read tstate->interp->ceval.gil_drop_request
        • RESET_GIL_DROP_REQUEST() (or COMPUTE_EVAL_BREAKER() ) called with tstate->interp
        • read tstate->async_exc
        • _PyEval_SignalAsyncExc() called with tstate->interp

    Some of these might be safe due to the logic but we need to verify that and fix the race on the rest. A race on any of these is unlikely, but possible if one thread is cleaning up interpreters/threads while another is creating them (as happens in test__xxsubinterpreters.

    As to the relationship with Windows, it looks like the sleep granularity (while waiting for a lock) there is much more coarse than it is with pthreads (e.g. on linux).

    Anyone is welcome to investigate further or propose solutions. I'll be looking at this more later on today.

    Some observations (from ceval_gil.c):

    • PyInterpreterState.ceval.gil_drop_request
      • a runtime-global boolean value (stored in an atomic int)
      • set by SET_GIL_DROP_REQUEST() and RESET_GIL_DROP_REQUEST()
        • RESET_GIL_DROP_REQUEST() (i.e. "clear") is used by drop_gil() and take_gil()
        • SET_GIL_DROP_REQUEST() is used only by take_gil()
        • both also touch the global eval_breaker
      • read by:
        • COMPUTE_EVAL_BREAKER() (if set then sets eval breaker)
        • drop_gil() (if set and calling tstate is still "holder" then call RESET_GIL_DROP_REQUEST() and wait)
        • take_gil() (if set then call RESET_GIL_DROP_REQUEST())
        • _Py_HandlePending() (if set then release and reacquire the GIL)
    • _PyRuntimeState.ceval.gil.last_holder
12. Fidget-Spinner commented on Jan 5, 2023

    

    Fidget-Spinner

    on Jan 5, 2023

    MemberAuthor

    More actions

    Thanks for your insightful comment! My own suspicion was that the GILState struct is a stack-allocated variable (it's a variable in a function rather than malloc-ed). So it's possible for one subinterpreter to outlive the GILState struct and segfault.
13. ericsnowcurrently commented on Jun 5, 2023

    

    ericsnowcurrently

    on Jun 5, 2023

    Member

    More actions

    Can you verify if this is fixed now? This may have been a case of gh-104341.
14. ericsnowcurrently

    moved this to Todo in Subinterpreterson Feb 26, 2024
15. ericsnowcurrently

    added

    pendingThe issue will be closed if no feedback is providedThe issue will be closed if no feedback is provided

    on Jul 8, 2024
16. picnixz commented on Dec 2, 2024

    

    picnixz

    on Dec 2, 2024

    Member

    More actions

    friendly reminder @Fidget-Spinner
17. sergey-miryanov commented on Jul 10, 2025

    

    sergey-miryanov

    on Jul 10, 2025

    Contributor

    More actions

    I ran it three times with over 100 iteration each time - no crashes occured.
    Run as .\python.bat -m test test__interpreters -F with debug build on 798f791.
18. encukou

    closed this as completedon Jul 10, 2025
19. github-project-automation

    moved this from Todo to Done in Subinterpreterson Jul 10, 2025