Skip to content

Conversation

@yoney
Copy link
Contributor

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

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

@mpagempage 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
Contributor

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

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

@mpagempage merged commit 65893c6 into python:mainJul 21, 2025
43 checks passed
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 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.
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 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.
@yoneyyoney deleted the ft_syslog branch October 19, 2025 03:02
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

@yoney@aisk@mpage@ZeroIntensity