session lock lifetime bug fix
array_hourly_0u at icloud.com
array_hourly_0u at icloud.com
Thu Jul 9 16:55:06 EDT 2026
Hi all,
I've opened PR #2264 (https://github.com/Gnucash/gnucash/pull/2264) to
fix a bug where qof_session_end() does not release the session lock
for a session that creates a store and is then ended without ever
loading it or making a data-changing save. The fix itself is a
one-liner as long as I have the design concepts correct.
What the lock actually protects:
-----------------------------------------
The gnclock row / .LCK file isn't protecting data -- it protects the
resource (the file, or the database). GnuCash's concurrency model is
single-writer: it reads the whole book into memory, edits in RAM, and
writes it back, with no row-level locking or conflict resolution. So
the lock means exactly one thing: I have this store open; nobody else
may open it until I let go.
Once you frame it as a resource lock, acquiring it in advance of any
real data is mandatory.
- The open (NORMAL_OPEN) case forces it: you must lock before load(),
because load() reads the entire store. If another writer could touch
it during your read you'd get a torn load; and since your in-memory
copy becomes authoritative and is written back at save(), you have to
hold the lock across the whole read-modify-write span.
Lock-before-read is the only race-free ordering.
- The create (NEW_STORE) case is the same principle: the store's
identity (its URI) exists the moment you begin(), even though its
content doesn't. Claiming that identity up front is what stops two
processes from both "creating" the same store and clobbering each
other. You lock the name/handle, not the bytes.
So the lock's natural lifetime is begin() to end() -- the span during
which you're attached to the resource -- and for a brand-new store
that span simply starts before any data exists. That isn't a wart;
it's the invariant.
What the bug actually revealed
-----------------------------------------
It revealed an asymmetry between acquire and release:
- Acquire was keyed on resource attachment -- the session's own
backend (m_backend), created in begin(). Correct.
- Release (the buggy end()) was keyed on the book's backend pointer
(qof_book_get_backend), which is only set when data flows: load(), or
a data-changing save().
So the lock's lifecycle was split across two different lifecycles: it
went up with resource attachment but came down with data attachment.
In the window where you attach but no data ever flows, those two
diverge, and the lock outlives its release path. The fix simply makes
release symmetric with acquire again -- both keyed on the session's
own backend.
The lock is unambiguously a connection-level concern, so it should be
driven entirely off the connection lifecycle.
Cheers,
Noah
More information about the gnucash-devel
mailing list