Make set_umask/get_umask thread safe#69842
Conversation
|
Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here's some information that may help as you continue your Salt journey. There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar. |
umask is process wide. Two threads overlapping in set_umask restore each other's saved value and the process umask stays changed for good. Hit this in salt-api: mk_key wraps key writes in set_umask(0o277) on every netapi request, rest_cherrypy runs requests in a thread pool, and once the umask stuck at 0o277 every jid file and thin bundle came out unwritable. Serialize both helpers with an RLock so nested use in a single thread still works. Fixes saltstack#66607
020589f to
1ad9ce2
Compare
2 participants
What does this PR do?
Serializes
set_umask/get_umaskwith an RLock. umask is process wide, so two threads overlapping inset_umaskrestore each other's saved value and the process umask stays changed for good.We hit this in salt-api:
mk_keywraps key writes inset_umask(0o277)on every netapi request and rest_cherrypy runs requests in a thread pool. Once the umask stuck at 0o277, every jid file and thin bundle came out unwritable and everyclient=sshrequest 500'd until a restart. Confirmed via/proc/<pid>/statusshowingUmask: 0277on the salt-api process while the master sat at 0022.What issues does this PR fix or reference?
Fixes #66607
Previous Behavior
Concurrent
set_umask/get_umaskcalls race and can leave the process umask permanently changed.New Behavior
umask set/restore is serialized. Nested
set_umaskin a single thread still works (RLock).Merge requirements satisfied?
Commits signed with GPG?
No