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

Make socket module thread safe #128277

Open
kumaraditya303 opened this issue Dec 26, 2024 · 5 comments
Open

Make socket module thread safe #128277

kumaraditya303 opened this issue Dec 26, 2024 · 5 comments
Assignees
@kumaraditya303
Labels
extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir topic-free-threading type-feature A feature request or enhancement

Comments

@kumaraditya303
Copy link
Contributor

kumaraditya303 commented Dec 26, 2024 •
edited by bedevere-app bot
Loading

While working on #128002, I am seeing many tsan warning because of socket modules.
This issue tracks work for making it thread safe.

Linked PRs
@kumaraditya303 kumaraditya303 added stdlib Python modules in the Lib dir topic-free-threading labels Dec 26, 2024
@picnixz picnixz added extension-modules C modules in the Modules dir type-feature A feature request or enhancement labels Dec 27, 2024
@WolframAlph
Copy link
Contributor

Hi @kumaraditya303 @picnixz , is this one free to pick up or someone is working on it?

@picnixz
Copy link
Contributor

picnixz commented Dec 27, 2024

I think Kumar wants to do it. Since in-depth free-threading knowledge is needed and the task is likely to be long (namely, there will be probably multiple PRs which may need to be synchronized with #128002), we should first determine which parts are not thread-safe and how we can make them so.

@bedevere-app bedevere-app bot mentioned this issue Dec 27, 2024
Open
@kumaraditya303
Copy link
Contributor Author

Yeah, I had already started to work on this and you are correct it is needs to be synchronized with asyncio work.

@picnixz
Copy link
Contributor

picnixz commented Dec 27, 2024

(Just assigning you to the issue so that users know this without having to read the thread)

@kumaraditya303
Copy link
Contributor Author

Reproducer:

from threading import Thread
import socket

def main():
    r = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
    def f():
        while True:
            r.detach()

    def g():
        while True:
            r.close()
    t1 = Thread(target=f)
    t1.start()

    t2 = Thread(target=g)
    t2.start()
    t1.join()
    t2.join()

main()
❯ ./python main.py
==================
WARNING: ThreadSanitizer: data race (pid=37380)
  Write of size 4 at 0x7fb00c8e0a40 by thread T1:
    #0 sock_detach Modules/socketmodule.c:3404 (_socket.cpython-314td-x86_64-linux-gnu.so+0xfbf3) (BuildId: 2d3cf6ab9784ec68dfd508837a206256d4f5da1b)
    #1 method_vectorcall_NOARGS Objects/descrobject.c:447 (python+0x181820) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #2 _PyObject_VectorcallTstate Include/internal/pycore_call.h:167 (python+0x16a928) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #3 PyObject_Vectorcall Objects/call.c:327 (python+0x16aa71) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #4 _PyEval_EvalFrameDefault Python/generated_cases.c.h:960 (python+0x33a0ec) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #5 _PyEval_EvalFrame Include/internal/pycore_ceval.h:116 (python+0x3681b1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #6 _PyEval_Vector Python/ceval.c:1911 (python+0x368437) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #7 _PyFunction_Vectorcall Objects/call.c:413 (python+0x16a4a1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #8 _PyObject_VectorcallTstate Include/internal/pycore_call.h:167 (python+0x16ec05) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #9 method_vectorcall Objects/classobject.c:71 (python+0x16ef25) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #10 _PyVectorcall_Call Objects/call.c:273 (python+0x16c8e1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #11 _PyObject_Call Objects/call.c:348 (python+0x16cd2b) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #12 PyObject_Call Objects/call.c:373 (python+0x16cd8f) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #13 thread_run Modules/_threadmodule.c:346 (python+0x4d3151) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #14 pythread_wrapper Python/thread_pthread.h:242 (python+0x429d59) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)

  Previous read of size 4 at 0x7fb00c8e0a40 by thread T2:
    #0 _socket_socket_close_impl Modules/socketmodule.c:3380 (_socket.cpython-314td-x86_64-linux-gnu.so+0x775b) (BuildId: 2d3cf6ab9784ec68dfd508837a206256d4f5da1b)
    #1 _socket_socket_close Modules/clinic/socketmodule.c.h:32 (_socket.cpython-314td-x86_64-linux-gnu.so+0x1037a) (BuildId: 2d3cf6ab9784ec68dfd508837a206256d4f5da1b)
    #2 method_vectorcall_NOARGS Objects/descrobject.c:447 (python+0x181820) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #3 _PyObject_VectorcallTstate Include/internal/pycore_call.h:167 (python+0x16a928) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #4 PyObject_Vectorcall Objects/call.c:327 (python+0x16aa71) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #5 _PyEval_EvalFrameDefault Python/generated_cases.c.h:960 (python+0x33a0ec) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #6 _PyEval_EvalFrame Include/internal/pycore_ceval.h:116 (python+0x3681b1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #7 _PyEval_Vector Python/ceval.c:1911 (python+0x368437) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #8 _PyFunction_Vectorcall Objects/call.c:413 (python+0x16a4a1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #9 _PyObject_VectorcallTstate Include/internal/pycore_call.h:167 (python+0x16ec05) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #10 method_vectorcall Objects/classobject.c:71 (python+0x16ef25) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #11 _PyVectorcall_Call Objects/call.c:273 (python+0x16c8e1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #12 _PyObject_Call Objects/call.c:348 (python+0x16cd2b) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #13 PyObject_Call Objects/call.c:373 (python+0x16cd8f) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #14 thread_run Modules/_threadmodule.c:346 (python+0x4d3151) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #15 pythread_wrapper Python/thread_pthread.h:242 (python+0x429d59) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)

  Thread T1 'Thread-1 (f)' (tid=37392, running) created by main thread at:
    #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1022 (libtsan.so.2+0x5ac1a) (BuildId: 38097064631f7912bd33117a9c83d08b42e15571)
    #1 do_start_joinable_thread Python/thread_pthread.h:289 (python+0x42a1aa) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #2 PyThread_start_joinable_thread Python/thread_pthread.h:313 (python+0x42a3a0) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #3 ThreadHandle_start Modules/_threadmodule.c:431 (python+0x4d2ae6) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #4 do_start_new_thread Modules/_threadmodule.c:1794 (python+0x4d2cef) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #5 thread_PyThread_start_joinable_thread Modules/_threadmodule.c:1917 (python+0x4d2f5d) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #6 cfunction_call Objects/methodobject.c:551 (python+0x1ee9e6) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #7 _PyObject_MakeTpCall Objects/call.c:242 (python+0x16a6fc) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #8 _PyObject_VectorcallTstate Include/internal/pycore_call.h:165 (python+0x16a9b9) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #9 PyObject_Vectorcall Objects/call.c:327 (python+0x16aa71) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #10 _PyEval_EvalFrameDefault Python/generated_cases.c.h:1993 (python+0x340549) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #11 _PyEval_EvalFrame Include/internal/pycore_ceval.h:116 (python+0x3681b1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #12 _PyEval_Vector Python/ceval.c:1911 (python+0x368437) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #13 PyEval_EvalCode Python/ceval.c:658 (python+0x3685ac) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #14 run_eval_code_obj Python/pythonrun.c:1338 (python+0x40a462) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #15 run_mod Python/pythonrun.c:1423 (python+0x40a74d) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #16 pyrun_file Python/pythonrun.c:1256 (python+0x40b1a7) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #17 _PyRun_SimpleFileObject Python/pythonrun.c:491 (python+0x40d040) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #18 _PyRun_AnyFileObject Python/pythonrun.c:78 (python+0x40d240) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #19 pymain_run_file_obj Modules/main.c:410 (python+0x44544c) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #20 pymain_run_file Modules/main.c:429 (python+0x4455c0) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #21 pymain_run_python Modules/main.c:697 (python+0x44644c) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #22 Py_RunMain Modules/main.c:776 (python+0x446790) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #23 pymain_main Modules/main.c:806 (python+0x446847) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #24 Py_BytesMain Modules/main.c:830 (python+0x4469a6) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #25 main Programs/python.c:15 (python+0x84aeb) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)

  Thread T2 'Thread-2 (g)' (tid=37393, running) created by main thread at:
    #0 pthread_create ../../../../src/libsanitizer/tsan/tsan_interceptors_posix.cpp:1022 (libtsan.so.2+0x5ac1a) (BuildId: 38097064631f7912bd33117a9c83d08b42e15571)
    #1 do_start_joinable_thread Python/thread_pthread.h:289 (python+0x42a1aa) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #2 PyThread_start_joinable_thread Python/thread_pthread.h:313 (python+0x42a3a0) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #3 ThreadHandle_start Modules/_threadmodule.c:431 (python+0x4d2ae6) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #4 do_start_new_thread Modules/_threadmodule.c:1794 (python+0x4d2cef) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #5 thread_PyThread_start_joinable_thread Modules/_threadmodule.c:1917 (python+0x4d2f5d) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #6 cfunction_call Objects/methodobject.c:551 (python+0x1ee9e6) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #7 _PyObject_MakeTpCall Objects/call.c:242 (python+0x16a6fc) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #8 _PyObject_VectorcallTstate Include/internal/pycore_call.h:165 (python+0x16a9b9) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #9 PyObject_Vectorcall Objects/call.c:327 (python+0x16aa71) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #10 _PyEval_EvalFrameDefault Python/generated_cases.c.h:2195 (python+0x341ad5) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #11 _PyEval_EvalFrame Include/internal/pycore_ceval.h:116 (python+0x3681b1) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #12 _PyEval_Vector Python/ceval.c:1911 (python+0x368437) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #13 PyEval_EvalCode Python/ceval.c:658 (python+0x3685ac) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #14 run_eval_code_obj Python/pythonrun.c:1338 (python+0x40a462) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #15 run_mod Python/pythonrun.c:1423 (python+0x40a74d) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #16 pyrun_file Python/pythonrun.c:1256 (python+0x40b1a7) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #17 _PyRun_SimpleFileObject Python/pythonrun.c:491 (python+0x40d040) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #18 _PyRun_AnyFileObject Python/pythonrun.c:78 (python+0x40d240) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #19 pymain_run_file_obj Modules/main.c:410 (python+0x44544c) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #20 pymain_run_file Modules/main.c:429 (python+0x4455c0) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #21 pymain_run_python Modules/main.c:697 (python+0x44644c) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #22 Py_RunMain Modules/main.c:776 (python+0x446790) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #23 pymain_main Modules/main.c:806 (python+0x446847) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #24 Py_BytesMain Modules/main.c:830 (python+0x4469a6) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)
    #25 main Programs/python.c:15 (python+0x84aeb) (BuildId: d9db3c5867c977522c3be92604eba81f48c82275)

SUMMARY: ThreadSanitizer: data race Modules/socketmodule.c:3404 in sock_detach

The sock_fd member of PySocketSockObject needs to be accessed by atomics because it can be accessed and modified concurrently by thread which may even not be holding the GIL. See also #116912

I think the fix would be make it thread safe by adding using relaxed atomics for sock_fd member.

This was referenced Dec 28, 2024
Open
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kumaraditya303 kumaraditya303

Labels
extension-modules C modules in the Modules dir stdlib Python modules in the Lib dir topic-free-threading type-feature A feature request or enhancement
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@picnixz @WolframAlph @kumaraditya303