Description

colesbury

opened on Jul 9, 2024

Issue body actions

Python allows multiple threads to concurrently access file descriptors through files and sockets. Race conditions at the Python level can lead to unexpected behavior, even with the global interpreter lock.

Thread sanitizer reports these races in some of our tests that exercise this behavior. We cannot fix these potential race conditions without introducing potential deadlocks.

For example, consider:

import threading

secrets = open("secrets.txt", "w");

def thread1():
    secrets.write("a secret")

def thread2():
    secrets.close()

def thread3():
    with open("log.txt", "w") as log:
        log.write("log message")

threading.Thread(target=thread1).start()
threading.Thread(target=thread2).start()
threading.Thread(target=thread3).start()

If you are particularly unlucky, then the file descriptor for secrets may be closed by thread2 and reused as the file descriptor for log just before thread1 writes to it. In other words, thread1 may write "a secret" to a completely different file or socket than it intended.

This can happen, even with the GIL, because the GIL is released around write() and close(). In other words, the following can happen:

1. The secrets.write() call releases the GIL, but before it actually calls the C write() function on the file descriptor....
2. The secrets.close() call closes the file descriptor
3. The open("log.txt", "w") re-uses the same file descriptor number
4. The secrets.write() call now write() on the wrong file descriptor

Note that we must release the GIL before calling write() or close() because these functions potentially block.

Reacted by Michael Droettboom, Kirill Podoprigora and sobolevn

Activity

1. colesbury

   added

   topic-IO

   on Jul 9, 2024
2. colesbury

   changed the title [-]Unfixable potential race conditions on file descriptor in CPython[/-] [+]Unfixable potential race conditions on file descriptors in CPython[/+] on Jul 9, 2024
3. serhiy-storchaka commented on Jul 10, 2024

   

   serhiy-storchaka

   on Jul 10, 2024

   Member

   More actions

   What concrete parts of the stdlib or tests have this race condition?

   There were some issues in multiprocessing, I tried to avoid introducing such kind of race condition.
4. colesbury

   added 3 commits that reference this issue on Jul 10, 2024

   pythongh-121592: Make select.poll() and related objects thread-safe

   5086314

   pythongh-121592: Make select.poll() and related objects thread-safe

   8b90a6c

   gh-121592: Make select.poll() and related objects thread-safe (#121594)

   44937d1
5. miss-islington

   added a commit that references this issue on Jul 11, 2024

   pythongh-121592: Make select.poll() and related objects thread-safe (p…

   2d82c2f
6. colesbury

   added a commit that references this issue on Jul 11, 2024

   [3.13] gh-121592: Make select.poll() and related objects thread-safe (G…

   3b5f8d2
7. colesbury commented on Jul 11, 2024

   

   colesbury

   on Jul 11, 2024

   ContributorAuthor

   More actions

   @serhiy-storchaka I don't think the stdlib has internal races on file descriptors. We have some in tests, but it may take me a bit to find them because we still have other race conditions reported by TSan, and it's easy to get them mixed up.

   test_socket.testClose - race between recv() and connect() reported by TSan. This seems benign to me -- unlike my original example, neither call creates or closes file descriptors -- but I'm not entirely sure.
8. estyxx

   added a commit that references this issue on Jul 17, 2024

   pythongh-121592: Make select.poll() and related objects thread-safe (p…

   d424609
9. colesbury commented on Mar 21, 2025

   

   colesbury

   on Mar 21, 2025

   ContributorAuthor

   More actions

   @kumaraditya303 fixed the file descriptor race in the asyncio tests:

   • gh-128002: fix tsan race in closing socket in test_sslproto #131399
   • gh-128002: fix tsan race in closing socket in test_ssl #131397

   I'm going to close this issue as I don't think there's anything actionable left to do.
10. colesbury

    closed this as not plannedon Mar 21, 2025

Issue actions