Skip to content

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
from
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
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions Dockerfile
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop

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

New targets shouldn't go in the Makefile.

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

np, will fix

Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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 DownExpand 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 DownExpand 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
View file
Open in desktop
Comment thread
ggoldber marked this conversation as resolved.
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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)

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

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 DownExpand 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)

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

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?

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

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?

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

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 DownExpand 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
View file
Open in desktop
Original file line numberDiff line numberDiff 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 DownExpand 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