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.

Sign up for GitHub

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

Jump to bottom

Race between Py_SetRecursionLimit and _Py_LeaveRecursiveCallPy under free-threading #128717

Closed
hawkinsp opened this issue Jan 10, 2025 · 1 comment
Closed

Race between Py_SetRecursionLimit and _Py_LeaveRecursiveCallPy under free-threading #128717

hawkinsp opened this issue Jan 10, 2025 · 1 comment
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes topic-free-threading type-bug An unexpected behavior, bug, or error

Comments

@hawkinsp
Copy link
Contributor

hawkinsp commented Jan 10, 2025 •
edited by bedevere-app bot
Loading

Bug report

Bug description:

Under Python 3.13 at commit 65da5db, we see the following TSAN race:

import concurrent.futures
import functools
import sys
import threading

num_threads = 8

def count(n):
  if n > 0:
    return count(n - 1) + 1
  return 0

def closure(b):
  b.wait()
  sys.setrecursionlimit(100)
  count(50)

with concurrent.futures.ThreadPoolExecutor(max_workers=num_threads) as executor:
  for _ in range(100):
    b = threading.Barrier(num_threads)
    for _ in range(num_threads):
      executor.submit(functools.partial(closure, b))

TSAN report:

WARNING: ThreadSanitizer: data race (pid=3579557)
  Write of size 4 at 0x72a40001e02c by thread T8:
    #0 Py_SetRecursionLimit /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:272:35 (python3.13+0x3dd302) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #1 sys_setrecursionlimit_impl /usr/local/google/home/phawkins/p/cpython/./Python/sysmodule.c:1319:5 (python3.13+0x4bab67) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #2 sys_setrecursionlimit /usr/local/google/home/phawkins/p/cpython/./Python/clinic/sysmodule.c.h:509:20 (python3.13+0x4bab67)
    #3 cfunction_vectorcall_O /usr/local/google/home/phawkins/p/cpython/Objects/methodobject.c:512:24 (python3.13+0x28a3c5) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #4 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x1eafea) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #5 PyObject_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c:327:12 (python3.13+0x1eafea)
    #6 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:813:23 (python3.13+0x3e290b) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #7 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3dea3a) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #8 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1807:12 (python3.13+0x3dea3a)
    #9 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb65f) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #10 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x572352) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #11 partial_vectorcall /usr/local/google/home/phawkins/p/cpython/./Modules/_functoolsmodule.c:252:16 (python3.13+0x572352)
    #12 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb2d3) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #13 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb2d3)
    #14 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb355) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #15 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:1355:26 (python3.13+0x3e4af2) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #16 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3dea3a) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #17 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1807:12 (python3.13+0x3dea3a)
    #18 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb65f) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #19 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x1ef62f) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #20 method_vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/classobject.c:70:20 (python3.13+0x1ef62f)
    #21 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb2d3) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #22 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb2d3)
    #23 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb355) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #24 thread_run /usr/local/google/home/phawkins/p/cpython/./Modules/_threadmodule.c:337:21 (python3.13+0x564a32) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #25 pythread_wrapper /usr/local/google/home/phawkins/p/cpython/Python/thread_pthread.h:243:5 (python3.13+0x4bddb7) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)

  Previous write of size 4 at 0x72a40001e02c by thread T7:
    #0 _Py_LeaveRecursiveCallPy /usr/local/google/home/phawkins/p/cpython/Python/ceval_macros.h:389:35 (python3.13+0x3f3993) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #1 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:5257:13 (python3.13+0x3f3993)
    #2 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3dea3a) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #3 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1807:12 (python3.13+0x3dea3a)
    #4 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb65f) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #5 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x572352) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #6 partial_vectorcall /usr/local/google/home/phawkins/p/cpython/./Modules/_functoolsmodule.c:252:16 (python3.13+0x572352)
    #7 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb2d3) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #8 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb2d3)
    #9 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb355) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #10 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:1355:26 (python3.13+0x3e4af2) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #11 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3dea3a) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #12 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1807:12 (python3.13+0x3dea3a)
    #13 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb65f) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #14 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x1ef62f) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #15 method_vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/classobject.c:70:20 (python3.13+0x1ef62f)
    #16 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb2d3) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #17 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb2d3)
    #18 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb355) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #19 thread_run /usr/local/google/home/phawkins/p/cpython/./Modules/_threadmodule.c:337:21 (python3.13+0x564a32) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)
    #20 pythread_wrapper /usr/local/google/home/phawkins/p/cpython/Python/thread_pthread.h:243:5 (python3.13+0x4bddb7) (BuildId: 19c569fa942016d8ac49d19fd40ccb1ddded939b)

Looks like the per-thread update of p->py_recursion_remaining is racy.

CPython versions tested on:

3.13

Operating systems tested on:

Linux

Linked PRs
@hawkinsp hawkinsp added the type-bug An unexpected behavior, bug, or error label Jan 10, 2025
@ZeroIntensity ZeroIntensity added topic-free-threading 3.13 bugs and security fixes 3.14 new features, bugs and security fixes labels Jan 10, 2025
@ZeroIntensity
Copy link
Member

Yeah, modifying other thread states as done in Py_SetRecursionLimit isn't particularly thread-safe. I think we should just make a stop-the-world pause before setting the values.

@bedevere-app bedevere-app bot mentioned this issue Jan 11, 2025
Merged
kumaraditya303 pushed a commit that referenced this issue Jan 12, 2025
@ZeroIntensity
gh-128717: Stop-the-world when setting the recursion limit (#128741)
f6c61bf
ZeroIntensity added a commit to ZeroIntensity/cpython that referenced this issue Jan 12, 2025
@ZeroIntensity
[3.13] pythongh-128717: Stop-the-world when setting the recursion lim…
6694b3b 
…it (pythonGH-128741)

(cherry picked from commit f6c61bf)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
ZeroIntensity added a commit to ZeroIntensity/cpython that referenced this issue Jan 12, 2025
@ZeroIntensity
[3.13] pythongh-128717: Stop-the-world when setting the recursion lim…
d3d3d5e 
…it (pythonGH-128741)

(cherry picked from commit f6c61bf)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
@bedevere-app bedevere-app bot mentioned this issue Jan 12, 2025
Merged
kumaraditya303 pushed a commit that referenced this issue Jan 14, 2025
@ZeroIntensity
[3.13] gh-128717: Stop-the-world when setting the recursion limit (GH…
1ec36a6 
…-128741) (#128757)

[3.13] gh-128717: Stop-the-world when setting the recursion limit (GH-128741)
(cherry picked from commit f6c61bf)

Co-authored-by: Peter Bierma <zintensitydev@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
3.13 bugs and security fixes 3.14 new features, bugs and security fixes topic-free-threading type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@hawkinsp @ZeroIntensity @kumaraditya303