From 17a49581733d406956c8bf553b2d1b5e2959356a Mon Sep 17 00:00:00 2001
From: Paul Licameli <Paul-Licameli@users.noreply.github.com>
Date: Mon, 20 Jul 2020 21:17:45 -0400
Subject: [PATCH] Unitary thread review (#623)

* Fix uninitialized members of ProjectFileIO...

... which fixes this problem I observed:

Opening a previously saved project, saving-as to another path, then exiting
Audacity, gives a progress dialog waiting for checkpoints to end, which doesn't
go away

* Remove two mutexes...

... One wasn't used at all, and another was only ever used by one thread, and
then not correctly unlocked for each locking on all possible paths.

* Values that the worker thread writes and main reads should be atomic

* Remember to close db connections even after failure to open
---
 src/ProjectFileIO.cpp | 13 +++++--------
 src/ProjectFileIO.h   |  8 +++-----
 2 files changed, 8 insertions(+), 13 deletions(-)

diff --git a/src/ProjectFileIO.cpp b/src/ProjectFileIO.cpp
index bd15b9b9f..f74787bbe 100644
--- a/src/ProjectFileIO.cpp
+++ b/src/ProjectFileIO.cpp
@@ -311,11 +311,8 @@ void ProjectFileIO::CheckpointThread()
          }
 
          // Capture the number of pages that need checkpointing and reset
-         mCheckpointCurrentPages = mCheckpointWaitingPages;
+         mCheckpointCurrentPages.store( mCheckpointWaitingPages );
          mCheckpointWaitingPages = 0;
-
-         // Lock out others while the checkpoint is running
-         mCheckpointActive.lock();
       }
 
       // Open another connection to the DB to prevent blocking the main thread.
@@ -334,10 +331,10 @@ void ProjectFileIO::CheckpointThread()
 
          // Reset
          mCheckpointCurrentPages = 0;
-
-         // Checkpoint is complete
-         mCheckpointActive.unlock();
       }
+      else
+         // Gotta close it anyway!
+         sqlite3_close( db );
    }
 
    return;
@@ -348,7 +345,7 @@ int ProjectFileIO::CheckpointHook(void *data, sqlite3 *db, const char *schema, i
    // Get access to our object
    ProjectFileIO *that = static_cast<ProjectFileIO *>(data);
 
-   // Qeuue the database pointer for our checkpoint thread to process
+   // Queue the database pointer for our checkpoint thread to process
    std::lock_guard<std::mutex> guard(that->mCheckpointMutex);
    that->mCheckpointWaitingPages = pages;
    that->mCheckpointCondition.notify_one();
diff --git a/src/ProjectFileIO.h b/src/ProjectFileIO.h
index 767ff49bc..83f5e34cb 100644
--- a/src/ProjectFileIO.h
+++ b/src/ProjectFileIO.h
@@ -229,11 +229,9 @@ private:
    std::thread mCheckpointThread;
    std::condition_variable mCheckpointCondition;
    std::mutex mCheckpointMutex;
-   std::mutex mCheckpointActive;
-   std::mutex mCheckpointClose;
-   std::atomic_bool mCheckpointStop;
-   uint64_t mCheckpointWaitingPages;
-   uint64_t mCheckpointCurrentPages;
+   std::atomic_bool mCheckpointStop{ false };
+   std::atomic< std::uint64_t > mCheckpointWaitingPages{ 0 };
+   std::atomic< std::uint64_t > mCheckpointCurrentPages{ 0 };
 
    friend SqliteSampleBlock;
    friend AutoCommitTransaction;