GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-117657: Make Py_TYPE and Py_SET_TYPE thread safe by Fidget-Spinner · Pull Request #120165 · python/cpython

Conversation

@Fidget-Spinner

@Fidget-Spinner Fidget-Spinner commented Jun 6, 2024 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: Make TSAN tests pass with the GIL disabled in free-threaded builds #117657

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Fidget-Spinner
Make Py_TYPE and Py_SET_TYPE thread safe
df767e5
@bedevere-app bedevere-app Bot added the awaiting core review label Jun 6, 2024
@bedevere-app bedevere-app Bot mentioned this pull request Jun 6, 2024
Make TSAN tests pass with the GIL disabled in free-threaded builds #117657
Closed
@Fidget-Spinner
Fidget-Spinner requested a review from colesbury June 6, 2024 15:50
@Fidget-Spinner Fidget-Spinner added the skip news label Jun 6, 2024
@Fidget-Spinner
fix Windows
2e9fda6
@Fidget-Spinner
Fidget-Spinner requested a review from DinoV June 6, 2024 16:32
@colesbury

colesbury commented Jun 6, 2024

Copy link
Copy Markdown
Contributor

Do you have a test / stack trace of the reported race? I don't think we should be modifying an object's type (Py_SET_TYPE) concurrently with accesses (Py_TYPE).
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner

Fidget-Spinner commented Jun 7, 2024 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

Example code:

class Foo:
    pass

class Bar:
    pass


import threading
import os

X = Foo()

def work():
    foo = X
    for _ in range(10000):
        foo.__class__ = Bar
        type(foo)
        foo.__class__ = Foo
        type(foo)

threads = []
for i in range(os.cpu_count() - 1):
    thread = threading.Thread(target=work)
    thread.start()
    threads.append(thread)

work()
for thread in threads:
    thread.join()

TSAN output:

SUMMARY: ThreadSanitizer: data race /home/ken/Documents/GitHub/cpython/./Include/object.h:249:16 in Py_TYPE
==================
==================
WARNING: ThreadSanitizer: data race (pid=16230)
  Read of size 8 at 0x7fffb4553038 by thread T5:
    #0 Py_TYPE /home/ken/Documents/GitHub/cpython/./Include/object.h:249:16 (python+0x2ead98) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #1 _PyObject_ManagedDictPointer /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_object.h:687:5 (python+0x2ead98)
    #2 _PyObject_GetManagedDict /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_object.h:694:34 (python+0x2ead98)
    #3 _PyObject_MaterializeManagedDict /home/ken/Documents/GitHub/cpython/Objects/dictobject.c:6724:26 (python+0x2ead98)
    #4 object_set_class /home/ken/Documents/GitHub/cpython/Objects/typeobject.c:6552:34 (python+0x39c72d) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #5 getset_set /home/ken/Documents/GitHub/cpython/Objects/descrobject.c:249:16 (python+0x26c494) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #6 _PyObject_GenericSetAttrWithDict /home/ken/Documents/GitHub/cpython/Objects/object.c:1694:19 (python+0x317b9d) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #7 PyObject_GenericSetAttr /home/ken/Documents/GitHub/cpython/Objects/object.c:1765:12 (python+0x317f4b) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #8 PyObject_SetAttr /home/ken/Documents/GitHub/cpython/Objects/object.c:1331:15 (python+0x31649b) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #9 _PyEval_EvalFrameDefault /home/ken/Documents/GitHub/cpython/Python/generated_cases.c.h:5506:27 (python+0x4a0acd) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #10 _PyEval_EvalFrame /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x47f793) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #11 _PyEval_Vector /home/ken/Documents/GitHub/cpython/Python/ceval.c:1819:12 (python+0x47f793)
    #12 _PyFunction_Vectorcall /home/ken/Documents/GitHub/cpython/Objects/call.c (python+0x2554bc) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #13 _PyObject_VectorcallTstate /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_call.h:168:11 (python+0x25b06b) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #14 method_vectorcall /home/ken/Documents/GitHub/cpython/Objects/classobject.c:70:20 (python+0x2596d5) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #15 _PyVectorcall_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:273:16 (python+0x254ea5) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #16 _PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:348:16 (python+0x2550bb) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #17 PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:373:12 (python+0x2552e7) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #18 thread_run /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:337:21 (python+0x679ff8) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #19 pythread_wrapper /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:243:5 (python+0x5bb3bb) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)

  Previous write of size 8 at 0x7fffb4553038 by thread T7:
    #0 Py_SET_TYPE /home/ken/Documents/GitHub/cpython/./Include/object.h:277:17 (python+0x39caba) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #1 object_set_class /home/ken/Documents/GitHub/cpython/Objects/typeobject.c:6579:9 (python+0x39caba)
    #2 getset_set /home/ken/Documents/GitHub/cpython/Objects/descrobject.c:249:16 (python+0x26c494) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #3 _PyObject_GenericSetAttrWithDict /home/ken/Documents/GitHub/cpython/Objects/object.c:1694:19 (python+0x317b9d) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #4 PyObject_GenericSetAttr /home/ken/Documents/GitHub/cpython/Objects/object.c:1765:12 (python+0x317f4b) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #5 PyObject_SetAttr /home/ken/Documents/GitHub/cpython/Objects/object.c:1331:15 (python+0x31649b) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #6 _PyEval_EvalFrameDefault /home/ken/Documents/GitHub/cpython/Python/generated_cases.c.h:5506:27 (python+0x4a0acd) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #7 _PyEval_EvalFrame /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x47f793) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #8 _PyEval_Vector /home/ken/Documents/GitHub/cpython/Python/ceval.c:1819:12 (python+0x47f793)
    #9 _PyFunction_Vectorcall /home/ken/Documents/GitHub/cpython/Objects/call.c (python+0x2554bc) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #10 _PyObject_VectorcallTstate /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_call.h:168:11 (python+0x25b06b) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #11 method_vectorcall /home/ken/Documents/GitHub/cpython/Objects/classobject.c:70:20 (python+0x2596d5) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #12 _PyVectorcall_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:273:16 (python+0x254ea5) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #13 _PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:348:16 (python+0x2550bb) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #14 PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:373:12 (python+0x2552e7) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #15 thread_run /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:337:21 (python+0x679ff8) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
    #16 pythread_wrapper /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:243:5 (python+0x5bb3bb) (BuildId: 44878605c2e4a29d06162c0aee7cd12bdd186077)
...

Py_SET_TYPE:

==================
==================
WARNING: ThreadSanitizer: data race (pid=19676)
  Write of size 8 at 0x7fffb4553038 by thread T8:
    #0 Py_SET_TYPE /home/ken/Documents/GitHub/cpython/./Include/object.h:277:17 (python+0x39caba) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #1 object_set_class /home/ken/Documents/GitHub/cpython/Objects/typeobject.c:6579:9 (python+0x39caba)
    #2 getset_set /home/ken/Documents/GitHub/cpython/Objects/descrobject.c:249:16 (python+0x26c494) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #3 _PyObject_GenericSetAttrWithDict /home/ken/Documents/GitHub/cpython/Objects/object.c:1694:19 (python+0x317b9d) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #4 PyObject_GenericSetAttr /home/ken/Documents/GitHub/cpython/Objects/object.c:1765:12 (python+0x317f4b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #5 PyObject_SetAttr /home/ken/Documents/GitHub/cpython/Objects/object.c:1331:15 (python+0x31649b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #6 _PyEval_EvalFrameDefault /home/ken/Documents/GitHub/cpython/Python/generated_cases.c.h:5506:27 (python+0x4a0acd) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #7 _PyEval_EvalFrame /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x47f793) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #8 _PyEval_Vector /home/ken/Documents/GitHub/cpython/Python/ceval.c:1819:12 (python+0x47f793)
    #9 _PyFunction_Vectorcall /home/ken/Documents/GitHub/cpython/Objects/call.c (python+0x2554bc) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #10 _PyObject_VectorcallTstate /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_call.h:168:11 (python+0x25b06b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #11 method_vectorcall /home/ken/Documents/GitHub/cpython/Objects/classobject.c:70:20 (python+0x2596d5) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #12 _PyVectorcall_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:273:16 (python+0x254ea5) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #13 _PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:348:16 (python+0x2550bb) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #14 PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:373:12 (python+0x2552e7) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #15 thread_run /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:337:21 (python+0x679ff8) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #16 pythread_wrapper /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:243:5 (python+0x5bb3bb) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)

  Previous read of size 8 at 0x7fffb4553038 by thread T16:
    #0 Py_TYPE /home/ken/Documents/GitHub/cpython/./Include/object.h:249:16 (python+0x379aa5) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #1 type_vectorcall /home/ken/Documents/GitHub/cpython/Objects/typeobject.c:4411:16 (python+0x379aa5)
    #2 _PyObject_VectorcallTstate /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_call.h:168:11 (python+0x25358b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #3 PyObject_Vectorcall /home/ken/Documents/GitHub/cpython/Objects/call.c:327:12 (python+0x254fa0) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #4 _PyEval_EvalFrameDefault /home/ken/Documents/GitHub/cpython/Python/generated_cases.c.h:813:23 (python+0x4854f2) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #5 _PyEval_EvalFrame /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x47f793) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #6 _PyEval_Vector /home/ken/Documents/GitHub/cpython/Python/ceval.c:1819:12 (python+0x47f793)
    #7 _PyFunction_Vectorcall /home/ken/Documents/GitHub/cpython/Objects/call.c (python+0x2554bc) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #8 _PyObject_VectorcallTstate /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_call.h:168:11 (python+0x25b06b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #9 method_vectorcall /home/ken/Documents/GitHub/cpython/Objects/classobject.c:70:20 (python+0x2596d5) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #10 _PyVectorcall_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:273:16 (python+0x254ea5) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #11 _PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:348:16 (python+0x2550bb) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #12 PyObject_Call /home/ken/Documents/GitHub/cpython/Objects/call.c:373:12 (python+0x2552e7) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #13 thread_run /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:337:21 (python+0x679ff8) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #14 pythread_wrapper /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:243:5 (python+0x5bb3bb) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)

  Thread T8 (tid=19686, running) created by main thread at:
    #0 pthread_create <null> (python+0xdd37b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #1 do_start_joinable_thread /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:290:14 (python+0x5ba02b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #2 PyThread_start_joinable_thread /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:314:9 (python+0x5b9e4a) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #3 ThreadHandle_start /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:422:9 (python+0x679aff) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #4 do_start_new_thread /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:1798:9 (python+0x679aff)
    #5 thread_PyThread_start_joinable_thread /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:1921:14 (python+0x67858a) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #6 cfunction_call /home/ken/Documents/GitHub/cpython/Objects/methodobject.c:540:18 (python+0x30b0ac) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #7 _PyObject_MakeTpCall /home/ken/Documents/GitHub/cpython/Objects/call.c:242:18 (python+0x253d00) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #8 _PyObject_VectorcallTstate /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_call.h:166:16 (python+0x2535ed) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #9 PyObject_Vectorcall /home/ken/Documents/GitHub/cpython/Objects/call.c:327:12 (python+0x254fa0) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #10 _PyEval_EvalFrameDefault /home/ken/Documents/GitHub/cpython/Python/generated_cases.c.h:1505:19 (python+0x489513) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #11 _PyEval_EvalFrame /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x47f793) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #12 _PyEval_Vector /home/ken/Documents/GitHub/cpython/Python/ceval.c:1819:12 (python+0x47f793)
    #13 PyEval_EvalCode /home/ken/Documents/GitHub/cpython/Python/ceval.c:599:21 (python+0x47f40a) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #14 run_eval_code_obj /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:1292:9 (python+0x59d0ed) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #15 run_mod /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:1377:19 (python+0x59ccc6) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #16 pyrun_file /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:1210:15 (python+0x597afc) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #17 _PyRun_SimpleFileObject /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:459:13 (python+0x597afc)
    #18 _PyRun_AnyFileObject /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:77:15 (python+0x59725b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #19 pymain_run_file_obj /home/ken/Documents/GitHub/cpython/Modules/main.c:357:15 (python+0x5d70da) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #20 pymain_run_file /home/ken/Documents/GitHub/cpython/Modules/main.c:376:15 (python+0x5d70da)
    #21 pymain_run_python /home/ken/Documents/GitHub/cpython/Modules/main.c:639:21 (python+0x5d5fe1) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #22 Py_RunMain /home/ken/Documents/GitHub/cpython/Modules/main.c:718:5 (python+0x5d5fe1)
    #23 pymain_main /home/ken/Documents/GitHub/cpython/Modules/main.c:748:12 (python+0x5d64f9) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #24 Py_BytesMain /home/ken/Documents/GitHub/cpython/Modules/main.c:772:12 (python+0x5d6579) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #25 main /home/ken/Documents/GitHub/cpython/./Programs/python.c:15:12 (python+0x15a840) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)

  Thread T16 (tid=19694, running) created by main thread at:
    #0 pthread_create <null> (python+0xdd37b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #1 do_start_joinable_thread /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:290:14 (python+0x5ba02b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #2 PyThread_start_joinable_thread /home/ken/Documents/GitHub/cpython/Python/thread_pthread.h:314:9 (python+0x5b9e4a) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #3 ThreadHandle_start /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:422:9 (python+0x679aff) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #4 do_start_new_thread /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:1798:9 (python+0x679aff)
    #5 thread_PyThread_start_joinable_thread /home/ken/Documents/GitHub/cpython/./Modules/_threadmodule.c:1921:14 (python+0x67858a) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #6 cfunction_call /home/ken/Documents/GitHub/cpython/Objects/methodobject.c:540:18 (python+0x30b0ac) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #7 _PyObject_MakeTpCall /home/ken/Documents/GitHub/cpython/Objects/call.c:242:18 (python+0x253d00) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #8 _PyObject_VectorcallTstate /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_call.h:166:16 (python+0x2535ed) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #9 PyObject_Vectorcall /home/ken/Documents/GitHub/cpython/Objects/call.c:327:12 (python+0x254fa0) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #10 _PyEval_EvalFrameDefault /home/ken/Documents/GitHub/cpython/Python/generated_cases.c.h:1505:19 (python+0x489513) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #11 _PyEval_EvalFrame /home/ken/Documents/GitHub/cpython/./Include/internal/pycore_ceval.h:119:16 (python+0x47f793) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #12 _PyEval_Vector /home/ken/Documents/GitHub/cpython/Python/ceval.c:1819:12 (python+0x47f793)
    #13 PyEval_EvalCode /home/ken/Documents/GitHub/cpython/Python/ceval.c:599:21 (python+0x47f40a) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #14 run_eval_code_obj /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:1292:9 (python+0x59d0ed) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #15 run_mod /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:1377:19 (python+0x59ccc6) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #16 pyrun_file /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:1210:15 (python+0x597afc) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #17 _PyRun_SimpleFileObject /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:459:13 (python+0x597afc)
    #18 _PyRun_AnyFileObject /home/ken/Documents/GitHub/cpython/Python/pythonrun.c:77:15 (python+0x59725b) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #19 pymain_run_file_obj /home/ken/Documents/GitHub/cpython/Modules/main.c:357:15 (python+0x5d70da) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #20 pymain_run_file /home/ken/Documents/GitHub/cpython/Modules/main.c:376:15 (python+0x5d70da)
    #21 pymain_run_python /home/ken/Documents/GitHub/cpython/Modules/main.c:639:21 (python+0x5d5fe1) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #22 Py_RunMain /home/ken/Documents/GitHub/cpython/Modules/main.c:718:5 (python+0x5d5fe1)
    #23 pymain_main /home/ken/Documents/GitHub/cpython/Modules/main.c:748:12 (python+0x5d64f9) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #24 Py_BytesMain /home/ken/Documents/GitHub/cpython/Modules/main.c:772:12 (python+0x5d6579) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)
    #25 main /home/ken/Documents/GitHub/cpython/./Programs/python.c:15:12 (python+0x15a840) (BuildId: 63fa402fa027b5ecdee99d48cfd2b28a7d12ceb7)

SUMMARY: ThreadSanitizer: data race /home/ken/Documents/GitHub/cpython/./Include/object.h:277:17 in Py_SET_TYPE
==================
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner
add test
bda9545
@Fidget-Spinner
Fidget-Spinner requested a review from ericsnowcurrently as a code owner June 7, 2024 04:28
@Fidget-Spinner Fidget-Spinner added the topic-free-threading label Jun 7, 2024
@Fidget-Spinner
style nits
b49dc1a
@Fidget-Spinner

Fidget-Spinner commented Jun 7, 2024

Copy link
Copy Markdown
Member Author

Wow... the test case actually causes a segfault even with the fixes, not just a tsan warning.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
vstinner
vstinner approved these changes Jun 7, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting core review labels Jun 7, 2024
@vstinner

vstinner commented Jun 7, 2024

Copy link
Copy Markdown
Member

Wow... the test case actually causes a segfault even with the fixes, not just a tsan warning.

Well, Py_TYPE() returns a borrow reference which is a bad thing :-/ But it shouldn't matter in this case, since the type is not deleted by the test.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner Fidget-Spinner added the DO-NOT-MERGE label Jun 7, 2024
@Fidget-Spinner

Fidget-Spinner commented Jun 7, 2024

Copy link
Copy Markdown
Member Author

Can't merge this until we fix the actual crasher code #120198. It seems even default build is affected.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Fidget-Spinner
Fidget-Spinner commented Jun 8, 2024
View reviewed changes
Comment thread Include/internal/pycore_interp.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner
use stronger guarantees
07459a1
picnixz
picnixz reviewed Jun 9, 2024
View reviewed changes
Comment thread Include/internal/pycore_interp.h Resolved

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner @picnixz
Update Include/internal/pycore_interp.h
156908d 
Co-authored-by: Bénédikt Tran <[email protected]>
@Fidget-Spinner Fidget-Spinner mentioned this pull request Jun 10, 2024
gh-120198: Fix crash when two thread edit __class__ #120195
Merged
@Fidget-Spinner Fidget-Spinner removed the DO-NOT-MERGE label Jun 12, 2024
Fidget-Spinner added 2 commits June 12, 2024 16:39
@Fidget-Spinner
Merge remote-tracking branch 'upstream/main' into pytype_threadsafe
af62c36
@Fidget-Spinner
Lock critical section
9a33a78
@Fidget-Spinner
Fidget-Spinner requested a review from markshannon as a code owner June 12, 2024 08:56
vstinner
vstinner reviewed Jun 12, 2024
View reviewed changes
Comment thread Objects/typeobject.c
}
Py_BEGIN_CRITICAL_SECTION(self);
if (newto->tp_flags & Py_TPFLAGS_HEAPTYPE) {
Py_INCREF(newto);

@vstinner vstinner Jun 12, 2024

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it important to do the INCREF(newto) and DECREF(oldto) inside the critical section?

Or it it possible to restrict the critical section to the following code?

oldto = Py_TYPE(self);
Py_SET_TYPE(self, newto);

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Fidget-Spinner Fidget-Spinner Jun 12, 2024

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right. I can shrink it!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Fidget-Spinner @Zheaoli
shrink critical section
e43acf0 
Co-Authored-By: Nadeshiko Manju <[email protected]>
@Fidget-Spinner
Fidget-Spinner requested a review from vstinner June 12, 2024 12:13
vstinner
vstinner approved these changes Jun 12, 2024
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Since my previous review, object_set_class() got a critical section and it LGTM.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@Fidget-Spinner
Fidget-Spinner merged commit e16aed6 into python:main Jun 12, 2024
@bedevere-app bedevere-app Bot removed the awaiting merge label Jun 12, 2024
@Fidget-Spinner

Fidget-Spinner commented Jun 12, 2024

Copy link
Copy Markdown
Member Author

Thanks for the review Victor!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner Fidget-Spinner added the needs backport to 3.13 only security fixes label Jun 12, 2024
@miss-islington-app

miss-islington-app Bot commented Jun 12, 2024

Copy link
Copy Markdown

Thanks @Fidget-Spinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 12, 2024
@Fidget-Spinner @picnixz
@Zheaoli @miss-islington
pythongh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (pythonGH-1…
24281f4 
…20165)

(cherry picked from commit e16aed6)

Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Nadeshiko Manju <[email protected]>
@bedevere-app

bedevere-app Bot commented Jun 12, 2024

Copy link
Copy Markdown

GH-120403 is a backport of this pull request to the 3.13 branch.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the needs backport to 3.13 only security fixes label Jun 12, 2024
@Fidget-Spinner
Fidget-Spinner deleted the pytype_threadsafe branch June 12, 2024 12:43
Fidget-Spinner added a commit that referenced this pull request Jun 12, 2024
@miss-islington @Fidget-Spinner
@picnixz @Zheaoli
[3.13] gh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (GH-120165) (
91c4444 
GH-120403)

gh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (GH-120165)
(cherry picked from commit e16aed6)

Co-authored-by: Ken Jin <[email protected]>
Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Nadeshiko Manju <[email protected]>
@colesbury

colesbury commented Jun 17, 2024

Copy link
Copy Markdown
Contributor

I think we should address Py_TYPE and Py_SET_TYPE differently. I don't think the critical section in object_set_class is strong enough because the reads and uses of Py_TYPE() are not performed under the same lock. In other words, some other thread may still be operating on the object and it's old type incorrectly (in a way that was previously protected by the GIL).

object_set_class is a rare enough operation that I think that it should use a stop-the-world pause:

  • We should stop-the-world while swapping out the type in object_set_class. This ensures that any readers are in a safe spot -- i.e., they're not in a code segment that was previously protected by the GIL.
  • We should not use atomic operations in Py_TYPE and Py_SET_TYPE because they're not necessary with the the stop-the-world pause and they effectively suppress TSAN reported data races without really fixing the underlying race conditions.

The thread-safety guarantees should be something like:

  • object_set_class is thread-safe (i.e., my_object.__class__ = my_type from Python)
  • Py_SET_TYPE not thread-safe if the object might be concurrently accessed from other threads.
  • Py_TYPE safe as long as the type is only changed through object_set_class.

@Fidget-Spinner, would you like to work on this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@Fidget-Spinner

Fidget-Spinner commented Jun 17, 2024

Copy link
Copy Markdown
Member Author

@colesbury yeah I can revert the atomics in Py_TYPE and PY_SET_TYPE and stop the world instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mrahtz pushed a commit to mrahtz/cpython that referenced this pull request Jun 30, 2024
@Fidget-Spinner @picnixz
@Zheaoli @mrahtz
pythongh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (pythonGH-1…
bb59019 
…20165)

Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Nadeshiko Manju <[email protected]>
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@Fidget-Spinner @picnixz
@Zheaoli @estyxx
pythongh-117657: Make Py_TYPE and Py_SET_TYPE thread safe (pythonGH-1…
86e1fc4 
…20165)

Co-authored-by: Bénédikt Tran <[email protected]>
Co-authored-by: Nadeshiko Manju <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@picnixz picnixz picnixz left review comments
@vstinner vstinner vstinner approved these changes
@colesbury colesbury Awaiting requested review from colesbury
@DinoV DinoV Awaiting requested review from DinoV
@ericsnowcurrently ericsnowcurrently Awaiting requested review from ericsnowcurrently ericsnowcurrently is a code owner
@markshannon markshannon Awaiting requested review from markshannon markshannon is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

skip news topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@Fidget-Spinner @colesbury @vstinner @picnixz
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.