gnucash stable: Multiple changes pushed
John Ralls
jralls at code.gnucash.org
Sat Jul 11 14:05:09 EDT 2026
Updated via https://github.com/Gnucash/gnucash/commit/453a4fe8 (commit)
via https://github.com/Gnucash/gnucash/commit/5dc53303 (commit)
from https://github.com/Gnucash/gnucash/commit/13d8da31 (commit)
commit 453a4fe8868d10414bcdbdca28054fbc432debf1
Merge: 13d8da3116 5dc53303b5
Author: John Ralls <jralls at ceridwen.us>
Date: Sat Jul 11 11:03:53 2026 -0700
Merge Noah Noerr's 'fix-qofsession-end-releases-lock' into stable.
commit 5dc53303b5b7acfad89a5d32a13c1b7011c5b20b
Author: Noah R <Noerr at users.noreply.github.com>
Date: Thu Jul 9 12:10:39 2026 -0700
[engine] qof_session_end: release the lock via the session's backend
qof_session_end() is documented to release the session lock and shut the
backend connection but it does not release the session lock in all scenarios
(BUG). qof_session_end() looked the backend up via qof_book_get_backend().
The book's backend pointer is only set by load() or a data-changing save() (an
empty book's save() early-returns before attaching it); begin() sets only the
session's own m_backend. So a session that creates a store and ends it without
loading or making a real change had a backend on the session but not on the
book: end() found none, skipped session_end(), and never released the lock --
it was only released later at destroy().
Because the fault is in backend-agnostic engine code it affects every backend
that locks at begin(): the SQL/DBI backends leave a stale row in the gnclock
table, and the XML/file backend leaves an abandoned <file>.LCK sentinel (with
no data file, since XML data is only written at save). The store then appears
locked to the next opener.
End the session's own backend (m_backend, always set at begin()) instead. It
already equals the book's backend whenever the latter is set, so the change is
a no-op for loaded/saved sessions and additionally covers the
created-but-not-attached case.
Add regression tests for both backends, each of which begins a create-mode
session, ends it without a data-changing save, and checks the lock is gone:
- test-backend-dbi-basic.cpp (test_dbi_new_store_end_releases_lock): reopens
the store and asserts it is not ERR_BACKEND_LOCKED.
- test-load-backend.cpp (test_new_store_end_releases_lock): asserts the XML
<file>.LCK lockfile has been removed.
diff --git a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
index 00a6682542..7f3142ab18 100644
--- a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
+++ b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
@@ -438,6 +438,41 @@ test_dbi_store_and_reload (Fixture* fixture, gconstpointer pData)
qof_session_destroy (session_3);
}
+static void
+test_dbi_new_store_end_releases_lock (Fixture* fixture, gconstpointer pData)
+{
+ const gchar* url = (const gchar*)pData;
+ auto msg = "[GncDbiSqlConnection::unlock_database()] There was no lock entry in the Lock table";
+ auto loglevel = static_cast<GLogLevelFlags> (G_LOG_LEVEL_WARNING |
+ G_LOG_FLAG_FATAL);
+ TestErrorStruct* check = test_error_struct_new (nullptr, loglevel, msg);
+ fixture->hdlrs = test_log_set_fatal_handler (fixture->hdlrs, check,
+ (GLogFunc)test_checked_handler);
+ if (fixture->filename)
+ url = fixture->filename;
+
+ /* Create a new store and end the session without a data-changing save (an
+ * empty book's save() is a no-op that never attaches the backend to the
+ * book). */
+ auto book1{qof_book_new()};
+ auto session_1 = qof_session_new (book1);
+ qof_session_begin (session_1, url, SESSION_NEW_OVERWRITE);
+ g_assert_cmpint (qof_session_get_error (session_1), ==, ERR_BACKEND_NO_ERR);
+ qof_session_end (session_1);
+ g_assert_cmpint (qof_session_get_error (session_1), ==, ERR_BACKEND_NO_ERR);
+
+ /* end() must have released the lock: reopening the store must not fail with
+ * ERR_BACKEND_LOCKED. */
+ auto book2{qof_book_new()};
+ auto session_2 = qof_session_new (book2);
+ qof_session_begin (session_2, url, SESSION_NORMAL_OPEN);
+ g_assert_cmpint (qof_session_get_error (session_2), !=, ERR_BACKEND_LOCKED);
+
+ qof_session_end (session_2);
+ qof_session_destroy (session_2);
+ qof_session_destroy (session_1);
+}
+
static void
test_dbi_reparent_split_keeps_slots (Fixture* fixture, gconstpointer pData)
{
@@ -777,6 +812,8 @@ create_dbi_test_suite (const char* dbm_name, const char* url)
auto subsuite = g_strdup_printf ("%s/%s", suitename, dbm_name);
GNC_TEST_ADD (subsuite, "store_and_reload", Fixture, url, setup,
test_dbi_store_and_reload, teardown);
+ GNC_TEST_ADD (subsuite, "new_store_end_releases_lock", Fixture, url,
+ setup_memory, test_dbi_new_store_end_releases_lock, teardown);
GNC_TEST_ADD (subsuite, "reparent_split_keeps_slots", Fixture, url,
setup_memory, test_dbi_reparent_split_keeps_slots, teardown);
GNC_TEST_ADD (subsuite, "safe_save", Fixture, url, setup_memory,
diff --git a/libgnucash/backend/xml/test/test-load-backend.cpp b/libgnucash/backend/xml/test/test-load-backend.cpp
index 0cf924a5be..eac6d02f22 100644
--- a/libgnucash/backend/xml/test/test-load-backend.cpp
+++ b/libgnucash/backend/xml/test/test-load-backend.cpp
@@ -25,6 +25,7 @@
* 02110-1301, USA.
*/
#include <config.h>
+#include <filesystem>
#include "qof.h"
#include "cashobjects.h"
#include "test-stuff.h"
@@ -32,6 +33,34 @@
#define GNC_LIB_NAME "gncmod-backend-xml"
#define GNC_LIB_REL_PATH "xml"
+static void
+test_new_store_end_releases_lock (void)
+{
+ gchar *dir = g_dir_make_tmp ("gnc-xml-lock-XXXXXX", NULL);
+ if (!do_test (dir != NULL, "could not create a temp directory"))
+ return;
+ gchar *path = g_build_filename (dir, "book.gnucash", NULL);
+ gchar *lockfile = g_strconcat (path, ".LCK", NULL);
+ gchar *uri = g_strconcat ("xml://", path, NULL);
+ QofSession *session = qof_session_new (qof_book_new ());
+
+ qof_session_begin (session, uri, SESSION_NEW_STORE);
+ do_test (qof_session_get_error (session) == ERR_BACKEND_NO_ERR,
+ "creating a new XML store failed");
+ qof_session_end (session);
+ do_test (!g_file_test (lockfile, G_FILE_TEST_EXISTS),
+ "qof_session_end() did not remove the .LCK lockfile");
+ qof_session_destroy (session);
+
+ std::filesystem::remove (lockfile);
+ std::filesystem::remove (path);
+ std::filesystem::remove (dir);
+ g_free (uri);
+ g_free (lockfile);
+ g_free (path);
+ g_free (dir);
+}
+
int main (int argc, char** argv)
{
g_setenv ("GNC_UNINSTALLED", "1", TRUE);
@@ -40,6 +69,7 @@ int main (int argc, char** argv)
do_test (
qof_load_backend_library (GNC_LIB_REL_PATH, GNC_LIB_NAME),
" loading gnc-backend-xml GModule failed");
+ test_new_store_end_releases_lock ();
print_test_results ();
qof_close ();
exit (get_rv ());
diff --git a/libgnucash/engine/qofsession.cpp b/libgnucash/engine/qofsession.cpp
index 0afd47f791..32d6264a9c 100644
--- a/libgnucash/engine/qofsession.cpp
+++ b/libgnucash/engine/qofsession.cpp
@@ -335,9 +335,8 @@ void
QofSessionImpl::end () noexcept
{
ENTER ("sess=%p uri=%s", this, m_uri.c_str ());
- auto backend = qof_book_get_backend (m_book);
- if (backend != nullptr)
- backend->session_end();
+ if (m_backend != nullptr)
+ m_backend->session_end();
clear_error ();
m_uri.clear();
LEAVE ("sess=%p uri=%s", this, m_uri.c_str ());
Summary of changes:
.../backend/dbi/test/test-backend-dbi-basic.cpp | 37 ++++++++++++++++++++++
libgnucash/backend/xml/test/test-load-backend.cpp | 30 ++++++++++++++++++
libgnucash/engine/qofsession.cpp | 5 ++-
3 files changed, 69 insertions(+), 3 deletions(-)
More information about the gnucash-changes
mailing list