Skip to content

gh-127081: lock non-re-entrant *pwent calls - #132748

Merged
kumaraditya303 merged 6 commits into
python:mainfrom
duaneg:gh-127081-pwd
May 22, 2025
Merged

gh-127081: lock non-re-entrant *pwent calls#132748
kumaraditya303 merged 6 commits into
python:mainfrom
duaneg:gh-127081-pwd

Conversation

@duaneg

@duanegduaneg commented Apr 20, 2025

Copy link
Copy Markdown
Contributor

The libc setpwent, getpwent, and endpwent functions are not thread-safe. Protect them with mutexs in free-threading builds.

The libc setpwent, getpwent, and endpwent functions are not thread-safe.
Protect them with mutexs in free-threading builds.
@picnixzpicnixz changed the title gh-127081: lock non-re-entrant *pwent callsgh-127081: lock non-re-entrant *pwent callsApr 20, 2025

@ZeroIntensityZeroIntensity left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a critical section here. Py_CLEAR and Py_DECREF are re-entrant (as in, they can invoke the eval loop), so a mutex is prone to lock-ordering or re-entrancy deadlocks.

@ZeroIntensity

Copy link
Copy Markdown
Member

Actually, sorry, we need to fix subinterpreter thread-safety here too. We can't do that with a critical section. Could you adjust the mutex to go over solely the libc calls?

Comment threadModules/pwdmodule.c
Comment threadModules/pwdmodule.c Outdated
@duaneg

Copy link
Copy Markdown
ContributorAuthor

Py_CLEAR and Py_DECREF are re-entrant...Could you adjust the mutex to go over solely the libc calls?

Hmm, yeah, good point. I guess we can just defer the decrefs until exit.

Comment threadModules/pwdmodule.c Outdated
Comment threadModules/pwdmodule.c Outdated
Comment threadModules/pwdmodule.c Outdated
@kumaraditya303
kumaraditya303 self-requested a review May 21, 2025 16:51
Comment threadModules/pwdmodule.c
… they are
strongly referenced by that, so we can dec-ref them safely.
Comment threadModules/pwdmodule.c Outdated
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@duaneg@ZeroIntensity@picnixz@kumaraditya303