Fixes SIGSEGV and improve thread safety of journal.Reader class
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
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).
Ping. Is this repository alive?
The PR #147 will help fix the GH Action headache for this PR.
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.