Skip to content

Unfixable potential race conditions on file descriptors in CPython #121544

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

Closed
colesbury opened this issue Jul 9, 2024 · 3 comments
Closed

Unfixable potential race conditions on file descriptors in CPython #121544

colesbury opened this issue Jul 9, 2024 · 3 comments
Labels

Comments

@colesbury
Copy link
Contributor

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.

@colesbury colesbury changed the title Unfixable potential race conditions on file descriptor in CPython Unfixable potential race conditions on file descriptors in CPython Jul 9, 2024
@serhiy-storchaka
Copy link
Member

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.

colesbury added a commit to colesbury/cpython that referenced this issue Jul 10, 2024
This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(pythongh-121544).
colesbury added a commit to colesbury/cpython that referenced this issue Jul 10, 2024
This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(pythongh-121544).
colesbury added a commit that referenced this issue Jul 11, 2024
This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(gh-121544).
miss-islington pushed a commit to miss-islington/cpython that referenced this issue Jul 11, 2024
…ythonGH-121594)

This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(pythongh-121544).
(cherry picked from commit 44937d1)

Co-authored-by: Sam Gross <[email protected]>
colesbury added a commit that referenced this issue Jul 11, 2024
…H-121594) (#121623)

This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(gh-121544).
(cherry picked from commit 44937d1)

Co-authored-by: Sam Gross <[email protected]>
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…ython#121594)

This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(pythongh-121544).
@colesbury
Copy link
Contributor Author

@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.

estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…ython#121594)

This makes select.poll() and kqueue() objects thread-safe in the
free-threaded build. Note that calling close() concurrently with other
functions is still not thread-safe due to races on file descriptors
(pythongh-121544).
@colesbury
Copy link
Contributor Author

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

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

@colesbury colesbury closed this as not planned Won't fix, can't repro, duplicate, stale Mar 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants