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 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
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@behrmann behrmann Apr 6, 2026

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.

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
View file
Open in desktop
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
View file
Open in desktop
Copy link
Copy Markdown
Contributor

@behrmann behrmann Apr 6, 2026

Choose a reason for hiding this comment

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

I'm not sure anybody here uses docker, especially in the branded Docker way (instead of unbranded spec ones) we're more mkosi people, so these files are are wont to bit rot. Also, it's using the Makefile and we've switched to meson. I'd just drop this and the compose file.

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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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)
Copy link
Copy Markdown
Contributor

@behrmann behrmann Apr 6, 2026

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 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)
Copy link
Copy Markdown
Contributor

@behrmann behrmann Apr 6, 2026

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?

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
View file
Open in desktop
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