Activity

1. colesbury

   added

   topic-SSL

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

   on Jan 12, 2026
2. ZeroIntensity commented on Jan 12, 2026

   

   ZeroIntensity

   on Jan 12, 2026

   Member

   More actions

   load_cert_chain looks fixable. We should acquire tstate_mutex (via PySSL_BEGIN_ALLOW_THREADS) around the SSL calls.

   For test_thread_recv_while_main_thread_sends, you might be interested in the history here -- specifically #137583. In short, I tried to fix these races a while ago by adding locking around OpenSSL calls, but that ended up deadlocking libraries that attempted to write to a socket while another thread was reading from it. I can't think of any way to fix that without running into that problem again.
3. picnixz

   added

   extension-modulesC modules in the Modules dirC modules in the Modules dir

   on Jan 12, 2026
4. colesbury commented on Jan 13, 2026

   

   colesbury

   on Jan 13, 2026

   ContributorAuthor

   More actions

   Thanks for the pointers. I think that locking around the read and write calls is probably the right way to go. python-websockets will need to change their test code, but I think their code is currently broken. That's not the kind of change we should do in a bugfix release, but I think it's appropriate in a major release. cc @gpshead for his thoughts.

   Separately, I don't think merging Py_BEGIN_ALLOW_THREADS with the mutex acquisition in SSL code makes sense. The mutex should be held across all of the OpenSSL calls in an operation, while Py_BEGIN/END_ALLOW_THREADS is only for the blocking calls.
5. ZeroIntensity commented on Jan 13, 2026

   

   ZeroIntensity

   on Jan 13, 2026

   Member

   More actions

   I believe @aaugustin expressed the concern that it would be very difficult to implement websockets without the concurrent read/write problem.

   The mutex should be held across all of the OpenSSL calls in an operation, while Py_BEGIN/END_ALLOW_THREADS is only for the blocking calls.

   Yeah, you're right. I think my original impression was that OpenSSL calls are only done under Py_BEGIN_ALLOW_THREADS blocks, but that doesn't look correct in hindsight.
6. colesbury

   added 3 commits that reference this issue on Jan 13, 2026

   pythongh-143756: Avoid borrowed reference in SSL code

   ...

   28e9ed4

   pythongh-143756: Fix potential data race in SSLContext.load_cert_chain

   ...

   0968a23

   pythongh-143756: Fix potential data race in SSLContext.load_cert_chain

   ...

   4109a33
7. colesbury commented on Jan 13, 2026

   

   colesbury

   on Jan 13, 2026

   ContributorAuthor

   More actions

   I wonder if we can configure the OpenSSL BIO's to always be nonblocking and rely on PySSL_select for blocking. That way we could hold the mutex across the SSL_read_ex and SSL_write_ex calls, but not the PySSL_select calls.

   👍React with 👍1Reacted by Rudolf Polzer
8. aaugustin commented on Jan 16, 2026

   

   aaugustin

   on Jan 16, 2026

   Last edited by aaugustin

   More actions

   python-websockets will need to change their test code

   It is not a test problem, it's a production problem.

   Currently, it's possible to have a thread send a message (websocket.send(msg)) while another thread blocked waiting for the next message (msg = websocket.recv()). This translates directly to the same operations on a blocking socket — blocking on reads. I expect any other full-duplex protocol implemented with blocking reads to have the same problem.

   What would be the code change if that becomes impossible?

   For a single connection, I guess it would just be a matter of replacing a blocking recv by a non-blocking select followed by a non-blocking recv?

   For a server managing several connections, I suppose the best way is to select (or equivalent) all connections rather than once per connection, but that's a more significant change that doesn't fit well into websockets' architecture.
9. colesbury commented on Jan 16, 2026

   

   colesbury

   on Jan 16, 2026

   ContributorAuthor

   More actions

   It is not a test problem, it's a production problem.

   Thanks for the clarification.

   What would be the code change if that becomes impossible? For a single connection, I guess it would just be a matter of replacing a blocking recv by a non-blocking select followed by a non-blocking recv?

   You shouldn't need to change any code. My current plan is for CPython to basically do the thing you described. We'll internally configure the socket to be non-blocking so that the OpenSSL SSL_read_ex/SSL_write_ex calls are non-blocking. The SSLSocket recv()/send() calls will still block (depending on settimeout() and setblocking()) using select/poll.

   We only need the lock around the SSL_read_ex/SSL_write_ex calls, not select/poll.

   I don't think it'll be a big change -- this is already how settimeout() is implemented -- although I'm a little worried about implementation details leaking through something like socket.fileno().
10. aaugustin commented on Jan 18, 2026

    

    aaugustin

    on Jan 18, 2026

    More actions

    My current plan is for CPython to basically do the thing you described.

    That would be amazing 🤩
11. colesbury

    added 2 commits that reference this issue on Jan 22, 2026

    gh-143756: Fix potential data race in SSLContext.load_cert_chain (gh-…

    ...

    bcf9cb0

    gh-143756: Avoid borrowed reference in SSL code (gh-143816)

    ...

    ee4e14a
12. thunder-coding

    added 2 commits that reference this issue on Feb 15, 2026

    pythongh-143756: Fix potential data race in SSLContext.load_cert_chain (

    ...

    d74e5cd

    pythongh-143756: Avoid borrowed reference in SSL code (pythongh-143816)

    ...

    26ecab6
13. ZeroIntensity

    marked SSLSocket.recv/SSLSocket.send concurrency crash #151508 as a duplicate of this issue on Jun 15, 2026
14. divVerent commented on Jun 16, 2026

    

    divVerent

    on Jun 16, 2026

    Last edited by divVerent

    More actions

    Just as an example, a repro to get a crash out of the SSL_read/SSL_write data race is here, tsan or asan not needed:

    import os
    import socket
    import ssl
    import threading
    import time
    
    os.system("openssl req -x509 -newkey rsa:2048 -keyout key.pem -out cert.pem -nodes -subj /CN=localhost")
    
    def ssl_loop(func, stop_me):
        while not stop_me.is_set():
            try:
                func()
            except ssl.SSLError as e:
                if e.errno in (ssl.SSL_ERROR_WANT_READ, ssl.SSL_ERROR_WANT_WRITE):
                    continue
                break
            except Exception:
                break
    
    def run_ssl_server(server_sock, stop_event):
        context = ssl.create_default_context(ssl.Purpose.CLIENT_AUTH)
        context.load_cert_chain(certfile="cert.pem", keyfile="key.pem")
        conn, _ = server_sock.accept()
        ssl_conn = context.wrap_socket(conn, server_side=True)
        def server_step():
            c.sendall(b"S" * 4096)
            c.recv(4096)
        ssl_loop(server_step, stop_event)
    
    def try_to_crash():
        server_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        server_sock.bind(('127.0.0.1', 0))
        server_sock.listen(5)
        stop_event = threading.Event()
        threading.Thread(target=run_ssl_server, args=(server_sock, stop_event), daemon=True).start()
    
        client_sock = socket.socket(socket.AF_INET, socket.SOCK_STREAM)
        client_sock.connect(server_sock.getsockname())
        context = ssl.create_default_context(ssl.Purpose.SERVER_AUTH)
        context.check_hostname = False
        context.verify_mode = ssl.CERT_NONE
    
        ssl_client = context.wrap_socket(client_sock)
        ssl_client.settimeout(0.001)  # Can just retry if a short timeout is hit.
    
        # Spawn 10 reader and writer threads against the same SSL client,
        # and let them run for one second.
        threads = []
        for _ in range(10):
            t_read = threading.Thread(target=lambda: ssl_loop(lambda: ssl_client.recv(4096), stop_event))
            t_write = threading.Thread(target=lambda: ssl_loop(lambda: ssl_client.sendall(b"Y" * 4096), stop_event))
            threads.extend([t_read, t_write])
            t_read.start()
            t_write.start()
        time.sleep(1)
        stop_event.set()
        for t in threads:
            t.join(timeout=0.1)
        ssl_client.close()
        server_sock.close()
    
    for i in range(100):
        print(i)
        try_to_crash()
    print('did not crash')
    

    crashing with heap corruption:

    * thread #9, name = 'python3', stop reason = signal SIGSEGV: sent by kernel (SI_KERNEL)
       * frame #0: 0x00007ffff7ca497d libc.so.6`_int_malloc(av=0x00007fffc0000030, bytes=21) at malloc.c:4217:14
         frame #1: 0x00007ffff7ca57f2 libc.so.6`__libc_malloc2(bytes=21) at malloc.c:3458:12
         frame #2: 0x00007ffff6a613ee libcrypto.so.3`CRYPTO_malloc at mem.c:214:11
         frame #3: 0x00007ffff6a06eb5 libcrypto.so.3`err_set_debug(es=0x00007fffc0005f60, i=1, file="../ssl/record/methods/tls_common.c", line=873, fn="tls_get_more_records") at err_local.h:69:33 [inlined]
         frame #4: 0x00007ffff6a06e70 libcrypto.so.3`ERR_set_debug(file="../ssl/record/methods/tls_common.c", line=873, func="tls_get_more_records") at err_blocks.c:37:5
         frame #5: 0x00007ffff70dfcf9 libssl.so.3`tls_get_more_records at tls_common.c:921:9
         frame #6: 0x00007ffff70de29a libssl.so.3`tls_read_record at tls_common.c:1141:15
         frame #7: 0x00007ffff70d6535 libssl.so.3`ssl3_read_bytes at rec_layer_s3.c:698:19
         frame #8: 0x00007ffff706ac40 libssl.so.3`ssl3_read_internal.part.0 at s3_lib.c:5131:11
         frame #9: 0x00007ffff7079f1d libssl.so.3`SSL_read_ex at ssl_lib.c:2388:15
         frame #10: 0x00007ffff71b0c37 _ssl.cpython-313-x86_64-linux-gnu.so`_ssl__SSLSocket_read_impl(self=0x00007ffff674c6a0, len=4096, group_right_1=<unavailable>, buffer=0x00007fffda7fbaa0) at _ssl.c:2622:18
         frame #11: 0x00007ffff71b0b51 _ssl.cpython-313-x86_64-linux-gnu.so`_ssl__SSLSocket_read(self=0x00007ffff674c6a0, args=<unavailable>) at _ssl.c.h:544:20
         frame #12: 0x0000000000560d03 python3`method_vectorcall_VARARGS(func=<unavailable>, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at descrobject.c:324:24
         frame #13: 0x0000000000559723 python3`_PyObject_VectorcallTstate(tstate=0x0000000000e248c0, callable=0x00007ffff727cc70, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at pycore_call.h:168:11 [inlined]
         frame #14: 0x0000000000559708 python3`PyObject_Vectorcall(callable=0x00007ffff727cc70, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at call.c:327:12
         frame #15: 0x000000000056ea15 python3`_PyEval_EvalFrameDefault(tstate=<unavailable>, frame=<unavailable>, throwflag=<unavailable>) at generated_cases.c.h:1850:23
         frame #16: 0x00000000005dd5ea python3`_PyObject_VectorcallTstate(tstate=0x0000000000e248c0, callable=0x00007ffff6755da0, args=0x00007fffda7fbdd8, nargsf=1, kwnames=0x0000000000000000) at pycore_call.h:168:11 [inlined]
         frame #17: 0x00000000005dd5b7 python3`method_vectorcall(method=<unavailable>, args=<unavailable>, nargsf=<unavailable>, kwnames=<unavailable>) at classobject.c:71:20
         frame #18: 0x00000000006f7c10 python3`thread_run(boot_raw=0x0000000000d47a60) at _threadmodule.c:343:21
         frame #19: 0x000000000069f908 python3`pythread_wrapper(arg=<unavailable>) at thread_pthread.h:242:5
         frame #20: 0x00007ffff7c95dc9 libc.so.6`start_thread(arg=<unavailable>) at pthread_create.c:448:8
         frame #21: 0x00007ffff7d14d88 libc.so.6`__clone3 at clone3.S:78
    

    As such, this probably should be fixed - I guess the idea above to basically do the SSL_read/SSL_write calls under the mutex, but nonblocking, instead waiting prior to them with select, should work - with the one caveat that select indicating there are bytes may not imply the SSL_read will actually read something, but it may just update internal state (so if the contract to the caller is that at least 1 byte must be read, it will have to loop).

    Be aware that the mutex probably should be held up until the SSL_get_error call to ensure other threads can't interfere with the error value returned; so basically keep it with the `_PySSL_errno as a block, like it looks now.
15. kumaraditya303

    added 2 commits that reference this issue on Jul 10, 2026

    pythongh-143756: skip test_ssl_in_multiple_threads under TSAN

    ...

    c173e0d

    pythongh-143756: skip test_load_cert_chain_thread_safety under TSAN

    ...

    76ddcae
16. devdanzin commented on Aug 4, 2026

    

    devdanzin

    on Aug 4, 2026

    Member

    More actions

    One thing worth putting on the record before a fix is written, because it rules out an approach that looks obvious: @critical_section provides no mutual exclusion across a GIL-released region.

    Python/pystate.c:2323, in the detach path:

    if (tstate->critical_section != 0) {
        _PyCriticalSection_SuspendAll(tstate);
    }

    So any critical section held on entry is suspended for the duration of a Py_BEGIN_ALLOW_THREADS / PySSL_BEGIN_ALLOW_THREADS block, and re-acquired on reattach. For _ssl that means the 88 @critical_section clinic directives in the module do not protect the SSL_* call they wrap — only the Python-level bookkeeping on either side of it.

    Demonstrated without a sanitizer: two threads driven through a barrier end up simultaneously inside one SSL*, with OpenSSL reporting its own internal error.

    This matches the direction the thread is already going — the mutex has to be held across the OpenSSL calls, and Py_BEGIN/END_ALLOW_THREADS is only for the blocking ones — but it is worth being explicit that a fix expressed as "add @critical_section to these methods" would compile, look right, and change nothing.

    Same point applies to gh-150191.

    Written with AI assistance; the pystate.c coordinate was read at 4f3be1b5777 and the two-thread demonstration was run.
17. Flotiarenor

    added a commit that references this issue on Oct 12, 2026

    ci: 抑制 OpenSSL PBKDF2 的已知 TSan 竞争

    ...

    7b686b8