Skip to content

fix: prevent concurrent jrnl processes from silently clobbering each other's changes - #2097

Draft
alichtman wants to merge 27 commits into
mainfrom
fix/journal-concurrency-lock
Draft

alichtman wants to merge 27 commits into
mainfrom
fix/journal-concurrency-lock

Conversation

@alichtman

@alichtman alichtman commented Jul 15, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Patches for #1107.

image

Every command (append, --edit, --delete, --change-time, encrypt, decrypt, import) reads the whole journal into memory at open_journal() and overwrites the whole file at journal.write(). Two jrnl processes running against the same journal at once (e.g. two --edit sessions in different terminals) race on that read-modify-write span, and whichever writes last silently discards the other's changes.

  • open_journal() now acquires a per-journal lock file for the lifetime of the returned Journal (a new jrnl/lock.py, built on filelock). The lock file is a sibling of the journal path (e.g. journal.txt.lock), so Folder/DayOne directory journals get a lock file beside the directory rather than inside it.
  • A second process that can't acquire the lock within a few seconds fails fast with a clear error naming the PID and command line of the process holding it, instead of hanging indefinitely or silently overwriting the first process's work.
  • Callers release the lock via journal.release_lock() once done. jrnl/upgrade.py's legacy-journal-upgrade path needed an explicit release since it copies entries into a fresh Journal object that never inherits the lock from the one open_journal() returned (this was originally causing an intra-process self-deadlock, since the process could try to re-acquire its own held lock later in the same run — caught by the existing upgrade BDD tests).
  • filelock is promoted from a transitive dev dependency to a runtime dependency to provide the cross-platform locking primitive.

Builds on the atomic-write fix in #2096 (already merged).

Test plan

  • poetry run poe test (lint + full unit/BDD suite) passes
  • New unit tests: tests/unit/test_lock.py (lock acquire/release/timeout, PID/lock-path handling) and tests/unit/test_open_journal_lock.py (lock integration with open_journal())
  • Manually verified end-to-end: held the lock from a separate process, confirmed a concurrent jrnl invocation fails fast (~4s) with an error naming the holder's PID/command line, journal content stayed intact, and normal operation resumed cleanly (no leftover lock file) once the lock was released

@alichtman
alichtman marked this pull request as draft July 15, 2026 13:01
@alichtman
alichtman force-pushed the fix/journal-concurrency-lock branch 7 times, most recently from dbde804 to 52a7a8e Compare July 15, 2026 14:53
@alichtman
alichtman force-pushed the fix/journal-concurrency-lock branch 11 times, most recently from 7755f58 to 628dee7 Compare July 16, 2026 11:16
…other's changes

Every command (append, --edit, --delete, --change-time, encrypt,
decrypt, import) reads the whole journal into memory at open_journal()
and overwrites the whole file at journal.write(). Two processes
running against the same journal at once (e.g. two --edit sessions in
different terminals) would race on that read-modify-write span, and
whichever wrote last silently discarded the other's changes (#1107).

open_journal() now acquires a per-journal lock file (a sibling of the
journal path, e.g. journal.txt.lock, so Folder/DayOne directory
journals get a lock beside rather than inside them) for the lifetime
of the returned Journal, via the new jrnl/lock.py. A second process
that can't acquire the lock within a few seconds fails fast with a
clear error naming the PID and command line of the process holding it,
instead of hanging indefinitely or overwriting the first process's
work. Callers release the lock via journal.release_lock() once done;
jrnl/upgrade.py's legacy-journal-upgrade path needed an explicit
release since it copies entries into a fresh Journal object that never
inherits the lock from the one open_journal() returned.

filelock is promoted from a transitive dev dependency to a runtime one
to provide the cross-platform file locking primitive.
@alichtman
alichtman force-pushed the fix/journal-concurrency-lock branch from 628dee7 to 83c3631 Compare July 16, 2026 11:47
jrnl-bot and others added 8 commits July 21, 2026 21:55
The raw JSON header could appear user-editable in a text editor, when
it is not — editing it breaks decryption. Base64-encoding it makes
clear the header isn't meant to be hand-edited. Journals encrypted
with v3 before this change still have a raw JSON header; a fallback
path reads it directly, and the base64 format is written on the next
save. Adds unit and BDD coverage for both the new format and the
legacy fallback.
save_config() writes to get_config_path() (~/.config/jrnl/jrnl.yaml)
whenever it isn't given an explicit alt_config_path. Three call sites
persisting config changes back to disk — flush_pending_config_updates()
(config.py), and the in-place update branches of postconfig_encrypt()
and postconfig_decrypt() (commands.py) — called save_config() without
passing alt_config_path through, so any config write triggered while
using --config-file/--cf (most commonly a v1/v2 to v3 encryption
upgrade during journal.write()) silently clobbered the real config
file instead of the one passed via --config-file.

flush_pending_config_updates() was introduced in 94c7763 ("Add v3
encryption with per-file random salt", GHSA-rhx6-37mm-5q9r), which
is what made this regression user-visible again. The same gap in
postconfig_encrypt()/postconfig_decrypt() dates back to 631e08a
(2020), when those commands were first split into commands.py.

Thread args.config_file_path through all three call sites so writes
stay scoped to whichever config file was actually loaded. Also add
BDD coverage for the encrypt/decrypt upgrade path (encrypted_v1
legacy config over --config-file), and strengthen the two existing
--encrypt/--decrypt BDD scenarios with an `editor:` assertion: they
previously only checked the top-level `encrypt:` key, which the
force_local update never touches, so they passed even on the buggy
code.

Fixes #2099
Matches --version output, which already prefixes the version with v.
The --diagnostic output is what gets pasted into bug reports, so it
should be consistent.
renovate Bot and others added 18 commits July 21, 2026 22:19
Repo is switching its default branch from develop to main; updates CI
branch triggers, docs links, and contributing guide accordingly, plus
adds instructions for updating an existing local clone.
Backspacing over a multi-byte character (e.g. ß) in a canonical-mode
terminal only erases one byte, which can leave stray invalid UTF-8 in
the input. Reading that with strict UTF-8 decoding raised an uncaught
UnicodeDecodeError and lost the entire entry. Replace invalid bytes
instead of crashing.

Fixes #2082
get_text_from_editor left the tempfile on disk if subprocess.call
raised (e.g. misconfigured editor), since os.remove only ran after a
successful read. Wrap the write/invoke/read in try/finally so the
tempfile is always removed.
get_text_from_editor had no logging despite spawning a subprocess and
doing disk I/O. Add debug logs around tempfile creation/cleanup and
editor invocation, an error log when the editor is misconfigured, and
a debug log in read_template_file's FileNotFoundError handler for
parity with the existing debug log in get_template_path.
…other's changes

Every command (append, --edit, --delete, --change-time, encrypt,
decrypt, import) reads the whole journal into memory at open_journal()
and overwrites the whole file at journal.write(). Two processes
running against the same journal at once (e.g. two --edit sessions in
different terminals) would race on that read-modify-write span, and
whichever wrote last silently discarded the other's changes (#1107).

open_journal() now acquires a per-journal lock file (a sibling of the
journal path, e.g. journal.txt.lock, so Folder/DayOne directory
journals get a lock beside rather than inside them) for the lifetime
of the returned Journal, via the new jrnl/lock.py. A second process
that can't acquire the lock within a few seconds fails fast with a
clear error naming the PID and command line of the process holding it,
instead of hanging indefinitely or overwriting the first process's
work. Callers release the lock via journal.release_lock(), either
directly or via the context-manager protocol (`with
open_journal_with_lock(...) as journal:`). jrnl/upgrade.py's
legacy-journal-upgrade path holds each journal's lock via a
contextlib.ExitStack for the duration of the upgrade, through the
final write, since the copy into a fresh Journal object via
Journal.from_journal() doesn't inherit the lock from the one
open_journal_with_lock() returned.

filelock is promoted from a transitive dev dependency to a runtime
one to provide the cross-platform file locking primitive, and psutil
(already a transitive dependency via filelock/virtualenv) is promoted
alongside it for cross-platform lock-holder process lookup, keeping
its existing emscripten/cygwin exclusion.
Contention is almost always an interactive session (a forgotten --edit
in another terminal) that isn't going to release the lock within any
short wait, so the wait just delayed the "who's holding it" prompt for
no benefit. JRNL_LOCK_TIMEOUT now defaults to 0; still overridable for
scripted use where a grace period helps.

That timeout was also reused to wait for a killed holder to actually
exit and to retry the lock afterwards -- zeroing it broke that path,
since a kill signal needs a moment to take effect. Split those into a
separate, fixed KILL_RECOVERY_TIMEOUT_SECONDS.

Also trimmed a few overly long comments down to size.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants