Thread-unsafe libc functions
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 77.2k
- Forks
- 35.9k
- PR merge metrics
- PR metrics pending
Description
Bug report
There are a a few non thread-safe libc functions used in Python that can be an issue for free threading, isolated subinterpreters, or sometimes even with the GIL.
In https://github.com/python/cpython/issues/126316, @vstinner fixed the use setgrent / getgrent. It's probably a good time to look for other similar issues.
clang-tidy
clang-tidy has a concurrency-mt-unsafe check that looks for "known-to-be-unsafe functions".
clang-tidy notes
Prerequisites: install clang-tidy-18 and bear.
./configure -C --with-pydebug --disable-gil
# generate compile_commands.json
bear -- make -j
run-clang-tidy-18 -checks='-*,concurrency-mt-unsafe' -p .
Unsafe libc functions
localeconv(): not thread-safe, see glibc's manual. Isnl_langinfoa substitue?setlocalesetpwent,getpwent, andendpwentinpwdmodule.c. These are similar togrpmodule.cand can likely be addressed the same way.getservbyname,getservbyport,getprotobynameinModules/socketmodule.c: usegetservbyname_r, etc.? Note these thread-safety issues affect the default build because we release the GIL around the relevant calls.dbm_open,dbm_close, etc. inModules/_dbmmodule.cgetlogin: usegetlogin_rif available?
Unfixable by us?
getenv,setenv,unsetenv,putenv: environment modification isn't thread-safe andgetenvis used extensively in other C libraries, so putting a lock around our accesses doesn't do much. (Might finally be fixed in glibc, see https://inbox.sourceware.org/libc-alpha/cover.1722193092.git.fweimer@redhat.com/).login_tty: not sure it matters aslogin_ttyis usually called after afork()
Safe due to our usage
getc_unlocked, safe because we use it within aflockfile()call.mbrtowc()- safe as long as the passed inmbstate_t *is non-NULL, which is the case in CPython.
Safe in glibc
These functions are flagged by clang-tidy because they are not guaranteed to be safe by POSIX, but they are safe in glibc. It'd be nice to verify that they are safe in other libc implementations. I don't think it's worth changing them:
dlerror()strerror(): see https://man7.org/linux/man-pages/man3/strerror.3.htmlsystem(): see https://man7.org/linux/man-pages/man3/system.3.htmlreaddir(DIR *dirp): see https://man7.org/linux/man-pages/man3/readdir.3.html: "...in modern implementations (including the glibc implementation), concurrent calls to readdir() that specify different directory streams are thread-safe."wcstombs: see https://man7.org/linux/man-pages/man3/wcstombs.3.htmlnl_langinfo: https://www.man7.org/linux/man-pages/man3/nl_langinfo.3.html (as long as locale doesn't change)tcsendbreak,tcflow: see https://www.man7.org/linux/man-pages/man3/termios.3.html#ATTRIBUTESsetlogmask: safe since glibc 2.33strsignal: not documented as thread-safe, but uses glibc uses TLS internally since 2.32
Other
exit(): apparently concurrent calls toexit()are not thread-safe, but I don't think it matters for our usages.ptsname(): we already useptsname_r(), but the static analyzer gets confused
Linked PRs
- gh-132591
- gh-132748
- gh-132749
- gh-132750
- gh-132751
- gh-135097
- gh-135098
- gh-139996
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Review the linked PRs first, then inspect the clang-tidy concurrency-mt-unsafe results for Modules/pwdmodule.c, Modules/socketmodule.c, and Modules/_dbmmodule.c. Run the documented clang-tidy command after configuring with --disable-gil; done means each relevant libc use is addressed or its safety is justified.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- c, python
- Domain
- operating-systems
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 20/100