diff --git a/src/smessage.cpp b/src/smessage.cpp index a2dd83e..d48a3fa 100644 --- a/src/smessage.cpp +++ b/src/smessage.cpp @@ -450,59 +450,29 @@ bool SecMsgDB::Open(const char* pszMode) }; -class SecMsgBatchScanner : public rocksdb::WriteBatch::Handler -{ -public: - std::string needle; - bool* deleted; - std::string* foundValue; - bool foundEntry; - - SecMsgBatchScanner() : foundEntry(false) {} - - virtual void Put(const rocksdb::Slice& key, const rocksdb::Slice& value) override - { - if (key.ToString() == needle) - { - foundEntry = true; - *deleted = false; - *foundValue = value.ToString(); - }; - }; - - virtual void Delete(const rocksdb::Slice& key) override - { - if (key.ToString() == needle) - { - foundEntry = true; - *deleted = true; - }; - }; -}; - // When performing a read, if we have an active batch we need to check it first // before reading from the database, as the rest of the code assumes that once -// a database transaction begins reads are consistent with it. It would be good -// to change that assumption in future and avoid the performance hit, though in -// practice it does not appear to be large. +// a database transaction begins reads are consistent with it. +// +// Previously implemented via rocksdb::WriteBatch::Handler subclass, which fails +// to link against Ubuntu's librocksdb-dev (typeinfo for the Handler base class +// isn't exported there). The pendingBatch map is updated alongside every Put +// or Delete on activeBatch and answers ScanBatch queries directly. bool SecMsgDB::ScanBatch(const CDataStream& key, std::string* value, bool* deleted) const { if (!activeBatch) return false; - + *deleted = false; - SecMsgBatchScanner scanner; - scanner.needle = key.str(); - scanner.deleted = deleted; - scanner.foundValue = value; - rocksdb::Status s = activeBatch->Iterate(&scanner); - if (!s.ok()) - { - printf("SecMsgDB ScanBatch error: %s\n", s.ToString().c_str()); + auto it = pendingBatch.find(key.str()); + if (it == pendingBatch.end()) return false; - }; - - return scanner.foundEntry; + if (!it->second.has_value()) { + *deleted = true; + return true; + } + *value = *it->second; + return true; } bool SecMsgDB::TxnBegin() @@ -510,6 +480,7 @@ bool SecMsgDB::TxnBegin() if (activeBatch) return true; activeBatch = new rocksdb::WriteBatch(); + pendingBatch.clear(); return true; }; @@ -523,6 +494,7 @@ bool SecMsgDB::TxnCommit() rocksdb::Status status = pdb->Write(writeOptions, activeBatch); delete activeBatch; activeBatch = NULL; + pendingBatch.clear(); if (!status.ok()) { @@ -537,6 +509,7 @@ bool SecMsgDB::TxnAbort() { delete activeBatch; activeBatch = NULL; + pendingBatch.clear(); return true; }; @@ -602,9 +575,10 @@ bool SecMsgDB::WritePK(CKeyID& addr, CPubKey& pubkey) if (activeBatch) { activeBatch->Put(ssKey.str(), ssValue.str()); + pendingBatch[ssKey.str()] = ssValue.str(); return true; }; - + rocksdb::WriteOptions writeOptions; writeOptions.sync = true; rocksdb::Status s = pdb->Put(writeOptions, ssKey.str(), ssValue.str()); @@ -613,7 +587,7 @@ bool SecMsgDB::WritePK(CKeyID& addr, CPubKey& pubkey) printf("SecMsgDB write failure: %s\n", s.ToString().c_str()); return false; }; - + return true; }; @@ -746,9 +720,10 @@ bool SecMsgDB::WriteSmesg(unsigned char* chKey, SecMsgStored& smsgStored) if (activeBatch) { activeBatch->Put(ssKey.str(), ssValue.str()); + pendingBatch[ssKey.str()] = ssValue.str(); return true; }; - + rocksdb::WriteOptions writeOptions; writeOptions.sync = true; rocksdb::Status s = pdb->Put(writeOptions, ssKey.str(), ssValue.str()); @@ -757,7 +732,7 @@ bool SecMsgDB::WriteSmesg(unsigned char* chKey, SecMsgStored& smsgStored) printf("SecMsgDB write failed: %s\n", s.ToString().c_str()); return false; }; - + return true; }; @@ -792,9 +767,10 @@ bool SecMsgDB::EraseSmesg(unsigned char* chKey) if (activeBatch) { activeBatch->Delete(ssKey.str()); + pendingBatch[ssKey.str()] = std::nullopt; return true; }; - + rocksdb::WriteOptions writeOptions; writeOptions.sync = true; rocksdb::Status s = pdb->Delete(writeOptions, ssKey.str()); diff --git a/src/smessage.h b/src/smessage.h index 2244da7..41b98a6 100644 --- a/src/smessage.h +++ b/src/smessage.h @@ -4,6 +4,9 @@ #ifndef SEC_MESSAGE_H #define SEC_MESSAGE_H +#include +#include + #include #include @@ -323,7 +326,13 @@ public: rocksdb::DB *pdb; // points to the global instance rocksdb::WriteBatch *activeBatch; - + + // Parallel record of every pending write (value) or delete (nullopt) on + // activeBatch. Used by ScanBatch to answer "is this key in the active + // batch?" without iterating the WriteBatch via Handler — Ubuntu's + // librocksdb-dev hides typeinfo for rocksdb::WriteBatch::Handler so a + // subclass-based scan fails to link there. + std::map> pendingBatch; }; std::string getTimeString(int64_t timestamp, char *buffer, size_t nBuffer); diff --git a/src/txdb-rocksdb.cpp b/src/txdb-rocksdb.cpp index 57f15ee..620be59 100644 --- a/src/txdb-rocksdb.cpp +++ b/src/txdb-rocksdb.cpp @@ -32,6 +32,40 @@ namespace fs = std::filesystem; // the same way the LevelDB backend shares its txdb singleton. static rocksdb::DB* g_rocksdb = nullptr; +namespace { + +// rocksdb::DB::Open shipped a raw DB** overload for years; newer releases +// (Homebrew's macOS rocksdb 10.x) replaced it with std::unique_ptr*. +// SFINAE picks whichever overload the linked rocksdb actually has — +// `int` is preferred over `long`, so when DB** exists, the first overload +// wins; otherwise the unique_ptr fallback runs. +template +inline auto OpenRocksDBImpl(const rocksdb::Options& opts, const std::string& path, + T** dbptr, int) + -> decltype(rocksdb::DB::Open(opts, path, dbptr)) +{ + return rocksdb::DB::Open(opts, path, dbptr); +} + +template +inline rocksdb::Status OpenRocksDBImpl(const rocksdb::Options& opts, const std::string& path, + T** dbptr, long) +{ + std::unique_ptr tmp; + auto s = rocksdb::DB::Open(opts, path, &tmp); + if (s.ok()) *dbptr = tmp.release(); + return s; +} + +inline rocksdb::Status OpenRocksDB(const rocksdb::Options& opts, + const std::string& path, + rocksdb::DB** dbptr) +{ + return OpenRocksDBImpl(opts, path, dbptr, 0); +} + +} // anonymous namespace + static rocksdb::Options GetRocksOptions() { rocksdb::Options opts; @@ -61,7 +95,7 @@ static void open_rocksdb(rocksdb::Options& options, bool fRemoveOld = false) fs::create_directory(directory); printf("Opening RocksDB in %s\n", directory.string().c_str()); - rocksdb::Status status = rocksdb::DB::Open(options, directory.string(), &g_rocksdb); + rocksdb::Status status = OpenRocksDB(options, directory.string(), &g_rocksdb); if (!status.ok()) { throw runtime_error(strprintf("open_rocksdb(): error opening database: %s", status.ToString().c_str())); @@ -139,6 +173,7 @@ bool CRocksTxDB::TxnBegin() if (activeBatch) return true; activeBatch = new rocksdb::WriteBatch(); + pendingBatch.clear(); return true; } @@ -148,6 +183,7 @@ bool CRocksTxDB::TxnCommit() rocksdb::Status status = pdb->Write(rocksdb::WriteOptions(), activeBatch); delete activeBatch; activeBatch = nullptr; + pendingBatch.clear(); if (!status.ok()) { printf("ERROR: RocksDB batch commit failure: %s\n", status.ToString().c_str()); printf("ERROR: This may indicate disk full, corruption, or permissions issue.\n"); @@ -161,38 +197,12 @@ bool CRocksTxDB::TxnAbort() { delete activeBatch; activeBatch = nullptr; + pendingBatch.clear(); return true; } namespace { -// rocksdb::WriteBatch::Handler used to scan the active batch for a pending -// write/delete on a given key, the same way the LevelDB backend does. -class CRocksBatchScanner : public rocksdb::WriteBatch::Handler { -public: - std::string needle; - bool* deleted = nullptr; - std::string* foundValue = nullptr; - bool foundEntry = false; - - CRocksBatchScanner() = default; - - void Put(const rocksdb::Slice& key, const rocksdb::Slice& value) override { - if (key.ToString() == needle) { - foundEntry = true; - *deleted = false; - *foundValue = value.ToString(); - } - } - - void Delete(const rocksdb::Slice& key) override { - if (key.ToString() == needle) { - foundEntry = true; - *deleted = true; - } - } -}; - class CRocksDBIterator final : public CTxDBIteratorBase { public: explicit CRocksDBIterator(rocksdb::Iterator* pit) : pit(pit) {} @@ -214,15 +224,15 @@ bool CRocksTxDB::ScanBatch(const std::string& key, std::string* value, bool* del { assert(activeBatch); *deleted = false; - CRocksBatchScanner scanner; - scanner.needle = key; - scanner.deleted = deleted; - scanner.foundValue = value; - rocksdb::Status status = activeBatch->Iterate(&scanner); - if (!status.ok()) { - throw runtime_error(status.ToString()); + auto it = pendingBatch.find(key); + if (it == pendingBatch.end()) + return false; + if (!it->second.has_value()) { + *deleted = true; + return true; } - return scanner.foundEntry; + *value = *it->second; + return true; } bool CRocksTxDB::ReadRaw(const std::string& key, std::string& value) const @@ -250,6 +260,7 @@ bool CRocksTxDB::WriteRaw(const std::string& key, const std::string& value) { if (activeBatch) { activeBatch->Put(key, value); + pendingBatch[key] = value; return true; } rocksdb::Status status = pdb->Put(rocksdb::WriteOptions(), key, value); @@ -266,6 +277,7 @@ bool CRocksTxDB::EraseRaw(const std::string& key) return false; if (activeBatch) { activeBatch->Delete(key); + pendingBatch[key] = std::nullopt; return true; } rocksdb::Status status = pdb->Delete(rocksdb::WriteOptions(), key); diff --git a/src/txdb-rocksdb.h b/src/txdb-rocksdb.h index 1167f96..e4a168f 100644 --- a/src/txdb-rocksdb.h +++ b/src/txdb-rocksdb.h @@ -7,6 +7,10 @@ #include "txdb-base.h" +#include +#include +#include + #include #include #include @@ -47,6 +51,13 @@ private: rocksdb::Options options; int nVersion; + // Parallel record of every pending write (value) or delete (nullopt) on + // activeBatch. Used by ScanBatch to answer "is this key already in the + // active batch?" without iterating the WriteBatch via Handler — Ubuntu's + // librocksdb-dev hides typeinfo for rocksdb::WriteBatch::Handler so a + // subclass-based scan fails to link there. + std::map> pendingBatch; + bool ScanBatch(const std::string& key, std::string* value, bool* deleted) const; }; diff --git a/src/util_signal.h b/src/util_signal.h index fe9ec47..c170be4 100644 --- a/src/util_signal.h +++ b/src/util_signal.h @@ -19,7 +19,7 @@ * CSignal fan-out signal, operator() returns void. * CSignal returns std::optional from the most-recently * connected slot (mirroring signals2's last_value - * policy with optional fallback when no slots). + * policy with optional fallback when no slots are connected). * * connect(slot) returns a Connection token; call .disconnect() to unsubscribe. * @@ -34,7 +34,9 @@ namespace signal_detail { template struct SignalState { - std::map slots; + // Renamed from `slots` because Qt's MOC headers `#define slots` to expand + // empty, which strips the member name in any TU that pulls in . + std::map slot_map; std::size_t next_id = 0; }; @@ -47,7 +49,7 @@ public: void disconnect() { if (auto sp = m_state.lock()) { - sp->slots.erase(m_id); + sp->slot_map.erase(m_id); } m_state.reset(); } @@ -76,11 +78,11 @@ public: Connection connect(slot_type slot) { std::size_t id = ++m_state->next_id; - m_state->slots.emplace(id, std::move(slot)); + m_state->slot_map.emplace(id, std::move(slot)); return Connection(m_state, id); } - bool empty() const { return m_state->slots.empty(); } + bool empty() const { return m_state->slot_map.empty(); } protected: std::shared_ptr> m_state; @@ -97,7 +99,7 @@ public: void operator()(CallArgs&&... args) const { // Snapshot lets slots mutate connections during invocation. - auto snapshot = this->m_state->slots; + auto snapshot = this->m_state->slot_map; for (auto& kv : snapshot) { if (kv.second) kv.second(args...); } @@ -113,7 +115,7 @@ public: template std::optional operator()(CallArgs&&... args) const { - auto snapshot = this->m_state->slots; + auto snapshot = this->m_state->slot_map; std::optional result; for (auto& kv : snapshot) { if (kv.second) result = kv.second(args...);