python-systemd icon indicating copy to clipboard operation
python-systemd copied to clipboard

Fixes SIGSEGV and improve thread safety of journal.Reader class

Open sebres opened this issue 9 months ago • 4 comments

This PR fixes threaded issues of journal.Reader class and improves its thread-safety, using simple protection with ref-counter; It would avoid segfault by closing journal across threads.

The algorithm is simple, here short explanation illustrating the approach on an example of #143:

TH Action ref_count j_handle
... Reader created and has initial ref-count 1 1 valid
A Enter Reader_wait(), increment ref-count, release the lock (GIL), enter sd_journal_wait() 2 valid
A Leave sd_journal_wait(), acquiring the lock (GIL), decrement ref-count, not calling sd_journal_close() because of ref_count == 1, leave Reader_wait() 1 valid
- ... now thread A repeats the wait() however thread B invokes close() in-between ... - -
A Enter Reader_wait(), increment ref-count, release the lock (GIL), enter sd_journal_wait() 2 valid
B Enter Reader_close(), set closed flag, decrement ref-count, but don't close it because of ref_count > 0 1 valid
A Leave sd_journal_wait(), acquiring the lock (GIL), decrement ref-count, calling sd_journal_close() because of ref_count == 0, leave Reader_wait() 0 NULL

If thread B manages to close the handle before A enters wait mode, it is also safe, because then sd_journal_wait() would generate EINVARG and Reader.wait() raises an error ([Errno 22] Invalid argument).

Anyway, no segfault happens anymore and it is more or less thread-safe.

closes gh-143

sebres avatar Apr 01 '25 15:04 sebres

7c99a39e6dc40367b4bbd3173f3db9dcc799cc97 seems to fix the GHA flows now (completely different issue), so can be cherry-picked to main branch as it is (or I could make a separate PR if necessary).

sebres avatar Apr 02 '25 17:04 sebres

Ping. Is this repository alive?

sebres avatar Jun 04 '25 09:06 sebres

The PR #147 will help fix the GH Action headache for this PR.

hongquan avatar Jun 28 '25 12:06 hongquan

The PR #147 will help fix the GH Action headache for this PR.

Hmm... There is basically no problem to fix "for this PR" - as you can see all GHA checks are passed. Nothing about #147 (maybe it solves more, no time to review), just... actually everything work here in this PR as expected.

sebres avatar Jun 30 '25 11:06 sebres