Skip to content

gh-116738: Make syslog module thread-safe #136760

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

Merged
merged 2 commits into from
Jul 21, 2025
Merged

gh-116738: Make syslog module thread-safe #136760

merged 2 commits into from
Jul 21, 2025

Conversation

yoney
Copy link
Contributor

@yoney yoney commented Jul 18, 2025

Make the setlogmask() function in the syslog module thread-safe. These changes are relevant for scenarios where the GIL is disabled or when using subinterpreters.

  • The functions syslog(), openlog(), and closelog() in the syslog module are already handled for FT-Python using the @critical_section on the module. However, there might be an issue with syslog() for subinterpreters on macOS.

#ifdef __APPLE__
// gh-98178: On macOS, libc syslog() is not thread-safe
syslog(priority, "%s", message);
#else

I will check/test the macOS subinterpreter and create a separate PR if necessary.

cc: @mpage @colesbury

@yoney yoney marked this pull request as ready for review July 18, 2025 17:07
@mpage mpage requested review from colesbury, Yhg1s and mpage July 18, 2025 19:10
Copy link
Contributor

@mpage mpage left a comment

Choose a reason for hiding this comment

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

LGTM!

@aisk
Copy link
Member

aisk commented Jul 19, 2025

I'm wondering whether these functions are thread safe on other UNIX-like platforms, but I found that on some BSDs (OpenBSD1 and NetBSD) and on AIX2, some of these syslog related functions are not thread safe, and they provide alternative versions with the _r suffix.

I'm not sure whether we should add locks for these functions on these platforms, or change the implementations to the _r versions.

But these platforms are not marked as supported in PEP113, so I think this is may not be a big issue for now.

Footnotes

  1. https://man.freebsd.org/cgi/man.cgi?query=syslog&apropos=0&sektion=3&manpath=OpenBSD+7.7&arch=default&format=html

  2. https://www.ibm.com/docs/en/aix/7.3.0?topic=s-syslog-openlog-closelog-setlogmask-subroutine

  3. https://peps.python.org/pep-0011/

@mpage
Copy link
Contributor

mpage commented Jul 21, 2025

@aisk - I think the current implementation should be thread-safe on those platforms on both builds. The GIL provides thread safety on the default build; a critical section provides thread-safety on the free-threaded builds. There is one notable exception, syslog, which @yoney just pointed out to me releases the GIL/CS around the call to syslog:

#ifdef __APPLE__
// gh-98178: On macOS, libc syslog() is not thread-safe
syslog(priority, "%s", message);
#else
Py_BEGIN_ALLOW_THREADS;
syslog(priority, "%s", message);
Py_END_ALLOW_THREADS;
#endif

I think we can address that in a follow up, if needed.

@mpage mpage merged commit 65893c6 into python:main Jul 21, 2025
43 checks passed
Sign up for free to 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