FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Journald GIL release by ggoldber · Pull Request #170 · systemd/python-systemd · GitHub

/ python-systemd Public

Journald GIL release #170

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.

Sign up for GitHub

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

Open
ggoldber wants to merge 1 commit into
base: main
Choose a base branch
Could not load branches
Branch not found: {{ refName }}

Are you sure you want to change the base?

Some commits from the old base branch may be removed from the timeline, and old review comments may become outdated.
from
Conversation 9 Commits 1 Checks 17 Files changed
Open

Journald GIL release #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension .build  (1) .c  (1) .md  (1) .options  (1) .py  (1) .yml  (1) No extension  (2) All 7 file types selected
Viewed files
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Unified
Split
Hide whitespace
Diff view
Unified
Split
Hide whitespace
19 changes: 19 additions & 0 deletions Dockerfile
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
FROM python:3.11-slim

# Install compilation dependencies
RUN apt-get update && apt-get install -y \
gcc \
pkg-config \
libsystemd-dev \
make \
git \
meson \
jq \
&& rm -rf /var/lib/apt/lists/*

RUN pip install --no-cache-dir wheel build

WORKDIR /src

# Fix git ownership issue
RUN git config --global --add safe.directory /src
10 changes: 9 additions & 1 deletion Makefile
Show comments
View file
Open in desktop

behrmann Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

New targets shouldn't go in the Makefile.

ggoldber Apr 7, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

np, will fix
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,14 @@ all: build
build:
$(PYTHON) -m build -Cbuild-dir=$(BUILD_DIR)

wheel:
rm -rf $(BUILD_DIR)
$(PYTHON) -m build --wheel -Cbuild-dir=$(BUILD_DIR) -Csetup-args="-Djournal_unlock_gil=1"

wheel_gil:
rm -rf $(BUILD_DIR)
$(PYTHON) -m build --wheel -Cbuild-dir=$(BUILD_DIR) -Csetup-args="-Djournal_unlock_gil=0"

install:
$(PYTHON) -m pip install .

Expand All @@ -24,7 +32,7 @@ sign: dist/systemd-python-$(VERSION).tar.gz
gpg --detach-sign -a dist/systemd-python-$(VERSION).tar.gz

clean:
rm -rf $(BUILD_DIR) systemd/*.so systemd/*.py[co] *.py[co] systemd/__pycache__
rm -rf $(BUILD_DIR) systemd/*.so systemd/*.py[co] *.py[co] systemd/__pycache__ dist

distclean: clean
rm -rf dist MANIFEST
Expand Down
28 changes: 24 additions & 4 deletions README.md
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ is separated into a number of modules:
- `systemd.login` wraps parts of `libsystemd` used to query logged in users
and available seats and machines.


Installation
============

Expand Down Expand Up @@ -41,14 +42,33 @@ The project is also available on pypi as `systemd-python`:
To build from source
--------------------

On CentOS, RHEL, and Fedora:
### On CentOS, RHEL, and Fedora:

dnf install git python3-pip gcc python3-devel systemd-devel
pip3 install 'git+https://github.com/systemd/python-systemd.git#egg=systemd-python'

On Debian or Ubuntu:
### On Debian or Ubuntu:

make is mirrored into docker-compose, to turn the build independent of packages installed

mkdir dist
mkdir build

to run make all:

docker compose run --rm make

to run any make command

docker compose run --rm make $command

for example

docker compose run --rm make wheel
docker compose run --rm make clean
docker compose run --rm make build
docker compose run --rm make check

apt install libsystemd-{journal,daemon,login,id128}-dev gcc python3-dev pkg-config

Usage
=====
Expand Down Expand Up @@ -133,7 +153,7 @@ Show kernel ring buffer (`journalctl -k`):
print(entry['MESSAGE'])

Read entries in reverse (`journalctl _EXE=/usr/bin/vim -r`):

from systemd import journal
class ReverseReader(journal.Reader):
def __next__(self):
Expand Down
9 changes: 9 additions & 0 deletions docker-compose.yml
Show comments
View file
Open in desktop
Comment thread
ggoldber marked this conversation as resolved.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
services:
make:
build: .
volumes:
- .:/src
- ./build:/build
- ./dist:/dist
entrypoint: /usr/bin/make
command: all
1 change: 1 addition & 0 deletions meson.build
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ add_project_arguments(
'-D_GNU_SOURCE=1',
'-DPACKAGE_VERSION="@0@"'.format(meson.project_version()),
'-DLIBSYSTEMD_VERSION=@0@'.format(libsystemd_dep.version()),
'-DSD_JOURNAL_SENDV_UNLOCK_GIL=@0@'.format(get_option('journal_unlock_gil')),
language : 'c',
)

Expand Down
1 change: 1 addition & 0 deletions meson.options
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
# SPDX-License-Identifier: LGPL-2.1-or-later

option('docs', type : 'boolean', value : false)
option('journal_unlock_gil', type: 'integer', value: 1, description: 'unlock gil before sending log to journal')
19 changes: 19 additions & 0 deletions src/systemd/_journal.c
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,14 @@
#include "macro.h"
#include "pyutil.h"


#if defined(SD_JOURNAL_SENDV_UNLOCK_GIL) && (SD_JOURNAL_SENDV_UNLOCK_GIL == 1)

behrmann Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think the prefix SD_ should be reserved by systemd proper.
#define JOURNAL_SENDV_UNLOCK_GIL 1
#else
#define JOURNAL_SENDV_UNLOCK_GIL 0
#endif


PyDoc_STRVAR(journal_sendv__doc__,
"sendv('FIELD=value', 'FIELD=value', ...) -> None\n\n"
"Send an entry to the journal."
Expand Down Expand Up @@ -43,8 +51,14 @@ static PyObject* journal_sendv(PyObject *self _unused_, PyObject *args) {
iov[i].iov_len = length;
}

#if (JOURNAL_SENDV_UNLOCK_GIL == 1)

behrmann Apr 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Can you comment on the thread safety of this? sd_journal objects (as in the C object, not the Python object), may only be accessed by a single thread at a time, see systemd/systemd#39199. I'm not sure this is ensured here, e.g. if two Python threads have access to the same (Python) journal object, sharing the same underlying sd_journal object one, both could try to write to the journal here and while in Python land this is is still ordered by the GIL, once both have released the GIL isn't it still racy? Also, how does this look with three-threading builds of Python?

ggoldber Apr 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Hi, thank you for the review.
logger handlers are (unless manually disabled) protected by Handler.lock which is by default not None, so GIL is in general redundant, unless someone, for some reason disables this lock manually.

It seems like in no gil python these macros are still used, to yield thread execution context (I might be wrong, can dive deeper)

In my case I have a single producer/consumer logging handler with QueueHandler/ QueueListener.
So I do not need the GIL to be held within QueueListener outside the scope of Queue put (no_wait )/get

If you have concerns, I can make it configurable (at runtime, in addition to compile time), wdyt?

septatrix Jun 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Nothing guarantees that all writes go through a logging handler. One can also call this function directly from different threads.

This should either be made very explicit in the documentation or just be backed by a different lock
Py_BEGIN_ALLOW_THREADS
#endif
/* Send the iovector to the journal. */
r = sd_journal_sendv(iov, argc);
#if (JOURNAL_SENDV_UNLOCK_GIL == 1)
Py_END_ALLOW_THREADS
#endif
if (r < 0) {
errno = -r;
PyErr_SetFromErrno(PyExc_OSError);
Expand Down Expand Up @@ -111,6 +125,11 @@ PyMODINIT_FUNC PyInit__journal(void) {
return NULL;
}

if (PyModule_AddIntConstant(m, "__sendv_unlock_gil__", JOURNAL_SENDV_UNLOCK_GIL)) {
Py_DECREF(m);
return NULL;
}

return m;
}
REENABLE_WARNING;
11 changes: 10 additions & 1 deletion src/systemd/journal.py
Show comments
View file
Open in desktop
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from syslog import (LOG_EMERG, LOG_ALERT, LOG_CRIT, LOG_ERR,
LOG_WARNING, LOG_NOTICE, LOG_INFO, LOG_DEBUG)

from ._journal import __version__, sendv, stream_fd
from ._journal import __version__, sendv, stream_fd, __sendv_unlock_gil__
from ._reader import (_Reader, NOP, APPEND, INVALIDATE,
LOCAL_ONLY, RUNTIME_ONLY,
SYSTEM, SYSTEM_ONLY, CURRENT_USER,
Expand Down Expand Up @@ -538,6 +538,15 @@ def __init__(self, level=_logging.NOTSET, sender_function=send, **kwargs):
self.send = sender_function
self._extra = kwargs

@staticmethod
def gil_unlocked():
"""Return whether the journal send function is configured to unlock the GIL.

This is useful for logging handlers to determine whether they can safely
call the send function without risking deadlocks.
"""
return __sendv_unlock_gil__ == 1

@classmethod
def with_args(cls, config=None):
"""Create a JournalHandler with a configuration dictionary
Expand Down
Loading
Back | FazBrowse Home | New Git URL