From b98b3a87b0c1fbf3692c13e3b71b207cc96a4526 Mon Sep 17 00:00:00 2001 From: orignal Date: Wed, 8 May 2024 19:09:03 -0400 Subject: [PATCH] fixed race codition between RouterInfo's buffer persist and update --- libi2pd/NetDb.cpp | 19 ++++++++++-------- libi2pd/NetDb.hpp | 2 +- libi2pd/RouterInfo.cpp | 45 +++++++++++++++++++++++++----------------- libi2pd/RouterInfo.h | 18 ++++++++++++----- 4 files changed, 52 insertions(+), 32 deletions(-) diff --git a/libi2pd/NetDb.cpp b/libi2pd/NetDb.cpp index 4f0700a8..21eb8125 100644 --- a/libi2pd/NetDb.cpp +++ b/libi2pd/NetDb.cpp @@ -698,7 +698,7 @@ namespace data expirationTimeout = i2p::context.IsFloodfill () ? NETDB_FLOODFILL_EXPIRATION_TIMEOUT*1000LL : NETDB_MIN_EXPIRATION_TIMEOUT*1000LL + (NETDB_MAX_EXPIRATION_TIMEOUT - NETDB_MIN_EXPIRATION_TIMEOUT)*1000LL*NETDB_MIN_ROUTERS/total; - std::list > > saveToDisk; + std::list > > saveToDisk; std::list removeFromDisk; auto own = i2p::context.GetSharedRouterInfo (); @@ -711,7 +711,14 @@ namespace data if (it.second->GetBuffer ()) { // we have something to save - saveToDisk.push_back(std::make_pair(ident, it.second)); + std::shared_ptr buffer; + { + std::lock_guard l(m_RouterInfosMutex); // possible collision between DeleteBuffer and Update + buffer = it.second->GetSharedBuffer (); + it.second->DeleteBuffer (); + } + if (buffer && !it.second->IsUnreachable ()) // don't save bad router + saveToDisk.push_back(std::make_pair(ident, buffer)); it.second->SetUnreachable (false); } it.second->SetUpdated (false); @@ -800,15 +807,11 @@ namespace data } } - void NetDb::PersistRouters (std::list > >&& update, + void NetDb::PersistRouters (std::list > >&& update, std::list&& remove) { for (auto it: update) - { - it.second->SaveToFile (m_Storage.Path(it.first)); - std::lock_guard l(m_RouterInfosMutex); // possible collision between DeleteBuffer and Update - it.second->DeleteBuffer (); - } + RouterInfo::SaveToFile (m_Storage.Path(it.first), it.second); for (auto it: remove) m_Storage.Remove (it); } diff --git a/libi2pd/NetDb.hpp b/libi2pd/NetDb.hpp index c00c55b1..50f6b033 100644 --- a/libi2pd/NetDb.hpp +++ b/libi2pd/NetDb.hpp @@ -146,7 +146,7 @@ namespace data void Load (); bool LoadRouterInfo (const std::string& path, uint64_t ts); void SaveUpdated (); - void PersistRouters (std::list > >&& update, + void PersistRouters (std::list > >&& update, std::list&& remove); void Run (); // exploratory thread void Explore (int numDestinations); diff --git a/libi2pd/RouterInfo.cpp b/libi2pd/RouterInfo.cpp index 980412cb..5fbb7026 100644 --- a/libi2pd/RouterInfo.cpp +++ b/libi2pd/RouterInfo.cpp @@ -34,6 +34,7 @@ namespace data { if (len > size ()) len = size (); memcpy (data (), buf, len); + m_BufferLen = len; } RouterInfo::RouterInfo (): m_Buffer (nullptr) @@ -60,7 +61,7 @@ namespace data { m_Addresses = boost::make_shared(); // create empty list m_Buffer = buf; - m_BufferLen = len; + if (m_Buffer) m_Buffer->SetBufferLen (len); ReadFromBuffer (true); } else @@ -129,8 +130,8 @@ namespace data if (s.is_open ()) { s.seekg (0,std::ios::end); - m_BufferLen = s.tellg (); - if (m_BufferLen < 40 || m_BufferLen > MAX_RI_BUFFER_SIZE) + size_t bufferLen = s.tellg (); + if (bufferLen < 40 || bufferLen > MAX_RI_BUFFER_SIZE) { LogPrint(eLogError, "RouterInfo: File ", fullPath, " is malformed"); return false; @@ -138,7 +139,8 @@ namespace data s.seekg(0, std::ios::beg); if (!m_Buffer) m_Buffer = NewBuffer (); - s.read((char *)m_Buffer->data (), m_BufferLen); + s.read((char *)m_Buffer->data (), bufferLen); + m_Buffer->SetBufferLen (bufferLen); } else { @@ -163,11 +165,12 @@ namespace data m_IsUnreachable = true; return; } - m_RouterIdentity = NewIdentity (m_Buffer->data (), m_BufferLen); + size_t bufferLen = m_Buffer->GetBufferLen (); + m_RouterIdentity = NewIdentity (m_Buffer->data (), bufferLen); size_t identityLen = m_RouterIdentity->GetFullLen (); - if (identityLen >= m_BufferLen) + if (identityLen >= bufferLen) { - LogPrint (eLogError, "RouterInfo: Identity length ", identityLen, " exceeds buffer size ", m_BufferLen); + LogPrint (eLogError, "RouterInfo: Identity length ", identityLen, " exceeds buffer size ", bufferLen); m_IsUnreachable = true; return; } @@ -181,7 +184,7 @@ namespace data return; } // verify signature - int l = m_BufferLen - m_RouterIdentity->GetSignatureLen (); + int l = bufferLen - m_RouterIdentity->GetSignatureLen (); if (l < 0 || !m_RouterIdentity->Verify ((uint8_t *)m_Buffer->data (), l, (uint8_t *)m_Buffer->data () + l)) { LogPrint (eLogError, "RouterInfo: Signature verification failed"); @@ -191,7 +194,7 @@ namespace data } // parse RI std::stringstream str; - str.write ((const char *)m_Buffer->data () + identityLen, m_BufferLen - identityLen); + str.write ((const char *)m_Buffer->data () + identityLen, bufferLen - identityLen); ReadFromStream (str); if (!str) { @@ -625,22 +628,28 @@ namespace data return m_Buffer->data (); } - bool RouterInfo::SaveToFile (const std::string& fullPath) + bool RouterInfo::SaveToFile (const std::string& fullPath, std::shared_ptr buf) { - if (m_IsUnreachable) return false; // don't save bad router - if (!m_Buffer) - { - LogPrint (eLogWarning, "RouterInfo: Can't save, m_Buffer == NULL"); - return false; - } + if (!buf) return false; std::ofstream f (fullPath, std::ofstream::binary | std::ofstream::out); if (!f.is_open ()) { LogPrint (eLogError, "RouterInfo: Can't save to ", fullPath); return false; } - f.write ((char *)m_Buffer->data (), m_BufferLen); + f.write ((char *)buf->data (), buf->GetBufferLen ()); return true; + } + + bool RouterInfo::SaveToFile (const std::string& fullPath) + { + if (m_IsUnreachable) return false; // don't save bad router + if (!m_Buffer) + { + LogPrint (eLogWarning, "RouterInfo: Can't save, m_Buffer == NULL"); + return false; + } + return SaveToFile (fullPath, m_Buffer); } size_t RouterInfo::ReadString (char * str, size_t len, std::istream& s) const @@ -1108,7 +1117,7 @@ namespace data m_Buffer = NewBuffer (); if (len > m_Buffer->size ()) len = m_Buffer->size (); memcpy (m_Buffer->data (), buf, len); - m_BufferLen = len; + m_Buffer->SetBufferLen (len); } std::shared_ptr RouterInfo::NewBuffer () const diff --git a/libi2pd/RouterInfo.h b/libi2pd/RouterInfo.h index ad388611..ec7644d7 100644 --- a/libi2pd/RouterInfo.h +++ b/libi2pd/RouterInfo.h @@ -188,6 +188,13 @@ namespace data Buffer () = default; Buffer (const uint8_t * buf, size_t len); + + size_t GetBufferLen () const { return m_BufferLen; }; + void SetBufferLen (size_t len) { m_BufferLen = len; }; + + private: + + size_t m_BufferLen = 0; }; typedef std::array, eNumTransports> Addresses; @@ -272,18 +279,20 @@ namespace data const uint8_t * GetBuffer () const { return m_Buffer ? m_Buffer->data () : nullptr; }; const uint8_t * LoadBuffer (const std::string& fullPath); // load if necessary - size_t GetBufferLen () const { return m_BufferLen; }; + size_t GetBufferLen () const { return m_Buffer ? m_Buffer->GetBufferLen () : 0; }; + void DeleteBuffer () { m_Buffer = nullptr; }; + std::shared_ptr GetSharedBuffer () const { return m_Buffer; }; bool IsUpdated () const { return m_IsUpdated; }; void SetUpdated (bool updated) { m_IsUpdated = updated; }; bool SaveToFile (const std::string& fullPath); - + static bool SaveToFile (const std::string& fullPath, std::shared_ptr buf); + std::shared_ptr GetProfile () const; void DropProfile () { m_Profile = nullptr; }; bool HasProfile () const { return (bool)m_Profile; }; bool Update (const uint8_t * buf, size_t len); - void DeleteBuffer () { m_Buffer = nullptr; }; bool IsNewer (const uint8_t * buf, size_t len) const; /** return true if we are in a router family and the signature is valid */ @@ -300,7 +309,7 @@ namespace data RouterInfo (); uint8_t * GetBufferPointer (size_t offset = 0 ) { return m_Buffer->data () + offset; }; void UpdateBuffer (const uint8_t * buf, size_t len); - void SetBufferLen (size_t len) { m_BufferLen = len; }; + void SetBufferLen (size_t len) { if (m_Buffer) m_Buffer->SetBufferLen (len); }; void RefreshTimestamp (); CompatibleTransports GetReachableTransports () const { return m_ReachableTransports; }; void SetReachableTransports (CompatibleTransports transports) { m_ReachableTransports = transports; }; @@ -328,7 +337,6 @@ namespace data FamilyID m_FamilyID; std::shared_ptr m_RouterIdentity; std::shared_ptr m_Buffer; - size_t m_BufferLen; uint64_t m_Timestamp; // in milliseconds boost::shared_ptr m_Addresses; // TODO: use std::shared_ptr and std::atomic_store for gcc >= 4.9 bool m_IsUpdated, m_IsUnreachable, m_IsFloodfill;