gnucash maint: Bug 796759 - --add-price-quotes <sql file> leaves a lock on the file.

John Ralls jralls at code.gnucash.org
Sat Jul 14 20:09:35 EDT 2018


Updated	 via  https://github.com/Gnucash/gnucash/commit/43af50bd (commit)
	from  https://github.com/Gnucash/gnucash/commit/de927d53 (commit)



commit 43af50bd8aea764eb526b775a63b1df0488eda16
Author: John Ralls <jralls at ceridwen.us>
Date:   Sat Jul 14 17:09:22 2018 -0700

    Bug 796759 - --add-price-quotes <sql file> leaves a lock on the file.
    
    First, save isn't necessary if the book is dirty, so don't... but that
    means that the book has to be marked dirty after a session swap. No more
    laziness.
    
    Second, regardless of the outcome of inner_main_add_price_quotes the
    session must be destroyed to remove the lock.
    
    A couple of cleanups in QofSessionImpl::save as well: Rewrote the
    descriptive comment to reflect how it really works when the backend has
    gotten disconnected and removed the superfluous qof_book_set_backend
    with the backend that we'd *just gotten from the book*.

diff --git a/gnucash/gnome-utils/gnc-file.c b/gnucash/gnome-utils/gnc-file.c
index 0f19e5f..72dddbe 100644
--- a/gnucash/gnome-utils/gnc-file.c
+++ b/gnucash/gnome-utils/gnc-file.c
@@ -1536,11 +1536,7 @@ gnc_file_do_save_as (GtkWindow *parent, const char* filename)
     /* if we got to here, then we've successfully gotten a new session */
     /* close up the old file session (if any) */
     qof_session_swap_data (session, new_session);
-
-    /* XXX At this point, we should really mark the data in the new session
-     * as being 'dirty', since we haven't saved it at all under the new
-     * session. But I'm lazy...
-     */
+    qof_book_mark_session_dirty (qof_session_get_book (new_session));
 
     qof_event_resume();
 
diff --git a/gnucash/gnucash-bin.c b/gnucash/gnucash-bin.c
index 441fa2a..2514e49 100644
--- a/gnucash/gnucash-bin.c
+++ b/gnucash/gnucash-bin.c
@@ -585,8 +585,13 @@ inner_main_add_price_quotes(void *closure, int argc, char **argv)
     gnc_shutdown(0);
     return;
 fail:
-    if (session && qof_session_get_error(session) != ERR_BACKEND_NO_ERR)
-        g_warning("Session Error: %s", qof_session_get_error_message(session));
+    if (session)
+    {
+        if (qof_session_get_error(session) != ERR_BACKEND_NO_ERR)
+            g_warning("Session Error: %s",
+                      qof_session_get_error_message(session));
+        qof_session_destroy(session);
+    }
     qof_event_resume();
     gnc_shutdown(1);
 }
diff --git a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
index 7fb08a9..1447f39 100644
--- a/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
+++ b/libgnucash/backend/dbi/test/test-backend-dbi-basic.cpp
@@ -401,6 +401,7 @@ test_dbi_store_and_reload (Fixture* fixture, gconstpointer pData)
     g_assert (session_2 != NULL);
     g_assert_cmpint (qof_session_get_error (session_2), == , ERR_BACKEND_NO_ERR);
     qof_session_swap_data (fixture->session, session_2);
+    qof_book_mark_session_dirty (qof_session_get_book (session_2));
     qof_session_save (session_2, NULL);
     g_assert (session_2 != NULL);
     g_assert_cmpint (qof_session_get_error (session_2), == , ERR_BACKEND_NO_ERR);
@@ -459,6 +460,7 @@ test_dbi_safe_save (Fixture* fixture, gconstpointer pData)
         goto cleanup;
     }
     qof_session_swap_data (fixture->session, session_1);
+    qof_book_mark_session_dirty (qof_session_get_book (session_1));
     qof_session_save (session_1, NULL);
     /* Do a safe save */
     qof_session_safe_save (session_1, NULL);
@@ -532,6 +534,7 @@ test_dbi_version_control (Fixture* fixture, gconstpointer pData)
         goto cleanup;
     }
     qof_session_swap_data (fixture->session, sess);
+    qof_book_mark_session_dirty (qof_session_get_book (sess));
     qof_session_save (sess, NULL);
     sql_be = reinterpret_cast<decltype(sql_be)>(qof_session_get_backend (sess));
     book = qof_session_get_book (sess);
@@ -587,6 +590,7 @@ test_dbi_business_store_and_reload (Fixture* fixture, gconstpointer pData)
     session_2 = qof_session_new ();
     qof_session_begin (session_2, url, FALSE, TRUE, TRUE);
     qof_session_swap_data (fixture->session, session_2);
+    qof_book_mark_session_dirty (qof_session_get_book (session_2));
     qof_session_save (session_2, NULL);
 
     // Reload the session data
diff --git a/libgnucash/engine/qofsession.cpp b/libgnucash/engine/qofsession.cpp
index 0c57736..2898d5d 100644
--- a/libgnucash/engine/qofsession.cpp
+++ b/libgnucash/engine/qofsession.cpp
@@ -451,23 +451,21 @@ QofSessionImpl::is_saving () const noexcept
 void
 QofSessionImpl::save (QofPercentageFunc percentage_func) noexcept
 {
+    if (!qof_book_session_not_saved (m_book)) //Clean book, nothing to do.
+        return;
     m_saving = true;
     ENTER ("sess=%p book_id=%s", this, m_book_id.c_str ());
 
-    /* If there is a backend, and the backend is reachable
-    * (i.e. we can communicate with it), then synchronize with
-    * the backend.  If we cannot contact the backend (e.g.
-    * because we've gone offline, the network has crashed, etc.)
-    * then give the user the option to save to the local disk.
-    *
-    * hack alert -- FIXME -- XXX the code below no longer
-    * does what the words above say.  This needs fixing.
-    */
+    /* If there is a backend, the book is dirty, and the backend is reachable
+     * (i.e. we can communicate with it), then synchronize with the backend.  If
+     * we cannot contact the backend (e.g.  because we've gone offline, the
+     * network has crashed, etc.)  then raise an error so that the controlling
+     * dialog can offer the user a chance to save in a different way.
+     */
     auto backend = qof_book_get_backend (m_book);
     if (backend)
     {
-        /* if invoked as SaveAs(), then backend not yet set */
-        qof_book_set_backend (m_book, backend);
+
         backend->set_percentage(percentage_func);
         backend->sync(m_book);
         auto err = backend->get_error();
diff --git a/libgnucash/engine/test/test-qofsession.cpp b/libgnucash/engine/test/test-qofsession.cpp
index 38a7cc6..595eb62 100644
--- a/libgnucash/engine/test/test-qofsession.cpp
+++ b/libgnucash/engine/test/test-qofsession.cpp
@@ -192,6 +192,7 @@ TEST (QofSessionTest, save)
     qof_backend_register_provider (get_provider ());
     QofSession s;
     s.begin ("book1", false, false, false);
+    qof_book_mark_session_dirty (s.get_book ());
     s.save (nullptr);
     EXPECT_EQ (sync_called, true);
     qof_backend_unregister_all_providers ();



Summary of changes:
 gnucash/gnome-utils/gnc-file.c                       |  6 +-----
 gnucash/gnucash-bin.c                                |  9 +++++++--
 .../backend/dbi/test/test-backend-dbi-basic.cpp      |  4 ++++
 libgnucash/engine/qofsession.cpp                     | 20 +++++++++-----------
 libgnucash/engine/test/test-qofsession.cpp           |  1 +
 5 files changed, 22 insertions(+), 18 deletions(-)



More information about the gnucash-changes mailing list