Skip to content

Commit 4163093

Browse files
committed
wallet: use Mutex for g_sqlite_mutex instead of GlobalMutex
Using `Mutex` provides stronger guarantee than `GlobalMutex` wrt Clang's thread safety analysis. Thus it is better to reduce the usage of `GlobalMutex` in favor of `Mutex`. Using `Mutex` for `g_sqlite_mutex` is ok because its usage is limited in `wallet/sqlite.cpp` and it does not require propagating the negative annotations to not relevant code.
1 parent 3f1f5f6 commit 4163093

File tree

2 files changed

+16
-4
lines changed

2 files changed

+16
-4
lines changed

src/wallet/sqlite.cpp

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,9 +23,6 @@
2323
namespace wallet {
2424
static constexpr int32_t WALLET_SCHEMA_VERSION = 0;
2525

26-
static GlobalMutex g_sqlite_mutex;
27-
static int g_sqlite_count GUARDED_BY(g_sqlite_mutex) = 0;
28-
2926
static void ErrorLogCallback(void* arg, int code, const char* msg)
3027
{
3128
// From sqlite3_config() documentation for the SQLITE_CONFIG_LOG option:
@@ -83,6 +80,9 @@ static void SetPragma(sqlite3* db, const std::string& key, const std::string& va
8380
}
8481
}
8582

83+
Mutex SQLiteDatabase::g_sqlite_mutex;
84+
int SQLiteDatabase::g_sqlite_count = 0;
85+
8686
SQLiteDatabase::SQLiteDatabase(const fs::path& dir_path, const fs::path& file_path, const DatabaseOptions& options, bool mock)
8787
: WalletDatabase(), m_mock(mock), m_dir_path(fs::PathToString(dir_path)), m_file_path(fs::PathToString(file_path)), m_use_unsafe_sync(options.use_unsafe_sync)
8888
{
@@ -146,6 +146,8 @@ SQLiteDatabase::~SQLiteDatabase()
146146

147147
void SQLiteDatabase::Cleanup() noexcept
148148
{
149+
AssertLockNotHeld(g_sqlite_mutex);
150+
149151
Close();
150152

151153
LOCK(g_sqlite_mutex);

src/wallet/sqlite.h

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
#ifndef BITCOIN_WALLET_SQLITE_H
66
#define BITCOIN_WALLET_SQLITE_H
77

8+
#include <sync.h>
89
#include <wallet/db.h>
910

1011
#include <sqlite3.h>
@@ -63,7 +64,16 @@ class SQLiteDatabase : public WalletDatabase
6364

6465
const std::string m_file_path;
6566

66-
void Cleanup() noexcept;
67+
/**
68+
* This mutex protects SQLite initialization and shutdown.
69+
* sqlite3_config() and sqlite3_shutdown() are not thread-safe (sqlite3_initialize() is).
70+
* Concurrent threads that execute SQLiteDatabase::SQLiteDatabase() should have just one
71+
* of them do the init and the rest wait for it to complete before all can proceed.
72+
*/
73+
static Mutex g_sqlite_mutex;
74+
static int g_sqlite_count GUARDED_BY(g_sqlite_mutex);
75+
76+
void Cleanup() noexcept EXCLUSIVE_LOCKS_REQUIRED(!g_sqlite_mutex);
6777

6878
public:
6979
SQLiteDatabase() = delete;

0 commit comments

Comments
 (0)