GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-116738: Make syslog module thread-safe by yoney · Pull Request #136760 · python/cpython

Conversation

@yoney

@yoney yoney commented Jul 18, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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.

cpython/Modules/syslogmodule.c

Lines 242 to 245 in 03017a8

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

  • Linux man page (3): setlogmast()

cc: @mpage @colesbury

  • Issue: Audit all built-in modules for thread safety #116738

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@yoney
pythongh-116738: Make syslog module thread-safe
d2f43bd
@bedevere-app bedevere-app Bot mentioned this pull request Jul 18, 2025
Audit all built-in modules for thread safety #116738
Open
@yoney
yoney marked this pull request as ready for review July 18, 2025 17:07
@bedevere-app bedevere-app Bot added the awaiting review label Jul 18, 2025
@mpage
mpage requested review from Yhg1s, colesbury and mpage July 18, 2025 19:10
mpage
mpage approved these changes Jul 18, 2025
View reviewed changes

@mpage mpage left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread Lib/test/test_free_threading/test_syslog.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Jul 18, 2025
@yoney
pythongh-116738: Wrap with try/finally
b8102af
@ZeroIntensity ZeroIntensity added the topic-free-threading label Jul 19, 2025
@aisk

aisk commented Jul 19, 2025

Copy link
Copy Markdown
Member

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/ ↩
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mpage

mpage commented Jul 21, 2025

Copy link
Copy Markdown
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:

cpython/Modules/syslogmodule.c

Lines 242 to 249 in 5f9e38f

#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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@mpage
mpage merged commit 65893c6 into python:main Jul 21, 2025
@bedevere-app bedevere-app Bot removed the awaiting merge label Jul 21, 2025
taegyunkim pushed a commit to taegyunkim/cpython that referenced this pull request Aug 4, 2025
@yoney @taegyunkim
pythongh-116738: Make syslog module thread-safe (python#136760)
171c8f8 
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
@yoney @Agent-Hellboy
pythongh-116738: Make syslog module thread-safe (python#136760)
bce3498 
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.
@yoney
yoney deleted the ft_syslog branch October 19, 2025 03:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mpage mpage mpage approved these changes
@colesbury colesbury Awaiting requested review from colesbury
@Yhg1s Yhg1s Awaiting requested review from Yhg1s

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

topic-free-threading

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@yoney @aisk @mpage @ZeroIntensity
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.