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

Race between partial_vectorcall_fallback and _PyVectorcall_FunctionInline under free-threading #128050

Open
hawkinsp opened this issue Dec 18, 2024 · 3 comments
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 Dec 18, 2024

Bug report

Bug description:

Repro:

Run the following Python code under v3.13.1t with thread-sanitizer enabled.

import concurrent.futures
import functools
import threading

def f(a, b):
  return a + b

def closure(b, g):
  b.wait()
  g(77)

num_threads = 8
with concurrent.futures.ThreadPoolExecutor(max_workers=num_threads) as executor:
  for _ in range(10000):
    b = threading.Barrier(num_threads)
    g = functools.partial(f, a=42)
    for _ in range(num_threads):
      executor.submit(functools.partial(closure, b, g))

Get:

==================
WARNING: ThreadSanitizer: data race (pid=130482)
  Read of size 8 at 0x7ff6e09ed1a8 by thread T2:
    #0 _PyVectorcall_FunctionInline /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:129:5 (python3.13+0x1eaf8f) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #1 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:163:12 (python3.13+0x1eaf8f)
    #2 PyObject_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c:327:12 (python3.13+0x1eaf8f)
    #3 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:813:23 (python3.13+0x3e24fb) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #4 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3de62a) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #5 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1811:12 (python3.13+0x3de62a)
    #6 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb61f) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #7 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x571bb2) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #8 partial_vectorcall /usr/local/google/home/phawkins/p/cpython/./Modules/_functoolsmodule.c:252:16 (python3.13+0x571bb2)
    #9 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb293) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #10 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb293)
    #11 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb315) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #12 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:1355:26 (python3.13+0x3e46e2) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #13 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3de62a) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #14 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1811:12 (python3.13+0x3de62a)
    #15 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb61f) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #16 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x1ef5ef) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #17 method_vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/classobject.c:70:20 (python3.13+0x1ef5ef)
    #18 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb293) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #19 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb293)
    #20 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb315) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #21 thread_run /usr/local/google/home/phawkins/p/cpython/./Modules/_threadmodule.c:337:21 (python3.13+0x564292) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #22 pythread_wrapper /usr/local/google/home/phawkins/p/cpython/Python/thread_pthread.h:243:5 (python3.13+0x4bd637) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)

  Previous write of size 8 at 0x7ff6e09ed1a8 by thread T8:
    #0 partial_vectorcall_fallback /usr/local/google/home/phawkins/p/cpython/./Modules/_functoolsmodule.c:224:21 (python3.13+0x571d8b) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #1 partial_vectorcall /usr/local/google/home/phawkins/p/cpython/./Modules/_functoolsmodule.c:238:16 (python3.13+0x571a06) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #2 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x1eafaa) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #3 PyObject_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c:327:12 (python3.13+0x1eafaa)
    #4 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:813:23 (python3.13+0x3e24fb) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #5 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3de62a) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #6 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1811:12 (python3.13+0x3de62a)
    #7 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb61f) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #8 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x571bb2) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #9 partial_vectorcall /usr/local/google/home/phawkins/p/cpython/./Modules/_functoolsmodule.c:252:16 (python3.13+0x571bb2)
    #10 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb293) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #11 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb293)
    #12 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb315) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #13 _PyEval_EvalFrameDefault /usr/local/google/home/phawkins/p/cpython/Python/generated_cases.c.h:1355:26 (python3.13+0x3e46e2) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #14 _PyEval_EvalFrame /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_ceval.h:119:16 (python3.13+0x3de62a) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #15 _PyEval_Vector /usr/local/google/home/phawkins/p/cpython/Python/ceval.c:1811:12 (python3.13+0x3de62a)
    #16 _PyFunction_Vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/call.c (python3.13+0x1eb61f) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #17 _PyObject_VectorcallTstate /usr/local/google/home/phawkins/p/cpython/./Include/internal/pycore_call.h:168:11 (python3.13+0x1ef5ef) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #18 method_vectorcall /usr/local/google/home/phawkins/p/cpython/Objects/classobject.c:70:20 (python3.13+0x1ef5ef)
    #19 _PyVectorcall_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:273:16 (python3.13+0x1eb293) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #20 _PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:348:16 (python3.13+0x1eb293)
    #21 PyObject_Call /usr/local/google/home/phawkins/p/cpython/Objects/call.c:373:12 (python3.13+0x1eb315) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #22 thread_run /usr/local/google/home/phawkins/p/cpython/./Modules/_threadmodule.c:337:21 (python3.13+0x564292) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)
    #23 pythread_wrapper /usr/local/google/home/phawkins/p/cpython/Python/thread_pthread.h:243:5 (python3.13+0x4bd637) (BuildId: 9c1c16fb1bb8a435fa6fa4c6944da5d41f654e96)

partial_vectorcall_fallback sets vectorcall to nullptr if it decides to fall back to a non-vectorcall code path:

pto->vectorcall = NULL;

But this races with the code that reads an object's vectorcall in the core function call code:

memcpy(&ptr, (char *) callable + offset, sizeof(ptr));

CPython versions tested on:

3.13

Operating systems tested on:

Linux

@hawkinsp hawkinsp added the type-bug An unexpected behavior, bug, or error label Dec 18, 2024
@ZeroIntensity ZeroIntensity added topic-free-threading type-bug An unexpected behavior, bug, or error 3.13 bugs and security fixes 3.14 new features, bugs and security fixes and removed type-bug An unexpected behavior, bug, or error labels Dec 18, 2024
@ZeroIntensity
Copy link
Member

Looks like a race. Probably somewhat related to #127266, but this isn't a full type slot, it's an offset.

@hawkinsp
Copy link
Contributor Author

hawkinsp commented Dec 18, 2024

Here perhaps the best fix is to implement the missing vectorcall case in partial?

(Or just skip setting the pointer to nullptr in free-threading mode.)

It seems like a fairly benign race: all it does is prevent a fast-path from being taken again. But there's no such thing as a benign race.

@colesbury
Copy link
Contributor

I think we should set vectorcall once in the partial constructor, instead of sometimes setting it to NULL lazily when it's invoked.

I don't think there's a good enough reason to have a different implementation for the default build and free threading build. I don't think this sort of change will have any performance impact.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to 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
Projects
None yet
Development

No branches or pull requests

3 participants