From 29a86a17350b931c2b46aebd05ab99ada0b8494c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 5 May 2012 21:30:38 +0200 Subject: [PATCH 1/3] Add extra asserts to addrman --- src/addrman.cpp | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/addrman.cpp b/src/addrman.cpp index 345261e22..c1a0df6a4 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -107,9 +107,15 @@ void CAddrMan::SwapRandom(int nRndPos1, int nRndPos2) if (nRndPos1 == nRndPos2) return; + assert(nRndPos1 >= 0 && nRndPos2 >= 0); + assert(nRndPos1 < vRandom.size() && nRndPos2 < vRandom.size()); + int nId1 = vRandom[nRndPos1]; int nId2 = vRandom[nRndPos2]; + assert(mapInfo.count(nId1) == 1); + assert(mapInfo.count(nId2) == 1); + mapInfo[nId1].nRandomPos = nRndPos2; mapInfo[nId2].nRandomPos = nRndPos1; @@ -130,6 +136,7 @@ int CAddrMan::SelectTried(int nKBucket) int nTemp = vTried[nPos]; vTried[nPos] = vTried[i]; vTried[i] = nTemp; + assert(nOldest == -1 || mapInfo.count(nTemp) == 1); if (nOldest == -1 || mapInfo[nTemp].nLastSuccess < mapInfo[nOldest].nLastSuccess) nOldest = nTemp; } @@ -139,11 +146,13 @@ int CAddrMan::SelectTried(int nKBucket) int CAddrMan::ShrinkNew(int nUBucket) { + assert(nUBucket >= 0 && nUBucket < vvNew.size()); std::set &vNew = vvNew[nUBucket]; // first look for deletable items for (std::set::iterator it = vNew.begin(); it != vNew.end(); it++) { + assert(mapInfo.count(*it)); CAddrInfo &info = mapInfo[*it]; if (info.IsTerrible()) { @@ -168,11 +177,13 @@ int CAddrMan::ShrinkNew(int nUBucket) { if (nI == n[0] || nI == n[1] || nI == n[2] || nI == n[3]) { + assert(nOldest == -1 || mapInfo.count(*it) == 1); if (nOldest == -1 || mapInfo[*it].nTime < mapInfo[nOldest].nTime) nOldest = *it; } nI++; } + assert(mapInfo.count(nOldest) == 1); CAddrInfo &info = mapInfo[nOldest]; if (--info.nRefCount == 0) { @@ -189,6 +200,8 @@ int CAddrMan::ShrinkNew(int nUBucket) void CAddrMan::MakeTried(CAddrInfo& info, int nId, int nOrigin) { + assert(vvNew[nOrigin].count(nId) == 1); + // remove the entry from all new buckets for (std::vector >::iterator it = vvNew.begin(); it != vvNew.end(); it++) { @@ -197,6 +210,8 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId, int nOrigin) } nNew--; + assert(info.nRefCount == 0); + // what tried bucket to move the entry to int nKBucket = info.GetTriedBucket(nKey); std::vector &vTried = vvTried[nKBucket]; @@ -214,6 +229,7 @@ void CAddrMan::MakeTried(CAddrInfo& info, int nId, int nOrigin) int nPos = SelectTried(nKBucket); // find which new bucket it belongs to + assert(mapInfo.count(vTried[nPos]) == 1); int nUBucket = mapInfo[vTried[nPos]].GetNewBucket(nKey); std::set &vNew = vvNew[nUBucket]; @@ -385,6 +401,7 @@ CAddress CAddrMan::Select_(int nUnkBias) std::vector &vTried = vvTried[nKBucket]; if (vTried.size() == 0) continue; int nPos = GetRandInt(vTried.size()); + assert(mapInfo.count(vTried[nPos]) == 1); CAddrInfo &info = mapInfo[vTried[nPos]]; if (GetRandInt(1<<30) < fChanceFactor*info.GetChance()*(1<<30)) return info; @@ -402,6 +419,7 @@ CAddress CAddrMan::Select_(int nUnkBias) std::set::iterator it = vNew.begin(); while (nPos--) it++; + assert(mapInfo.count(*it) == 1); CAddrInfo &info = mapInfo[*it]; if (GetRandInt(1<<30) < fChanceFactor*info.GetChance()*(1<<30)) return info; @@ -481,6 +499,7 @@ void CAddrMan::GetAddr_(std::vector &vAddr) { int nRndPos = GetRandInt(vRandom.size() - n) + n; SwapRandom(n, nRndPos); + assert(mapInfo.count(vRandom[n]) == 1); vAddr.push_back(mapInfo[vRandom[n]]); } } From 56f1e912397de656988cf7898b5457892bce3c30 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 5 May 2012 21:22:55 +0200 Subject: [PATCH 2/3] Fix addrman crashes A function returned the element to remove from a bucket, instead of its position in that bucket. This function was only called when a tried bucket overflowed, which only happens after many outgoing connections have been made. Closes: #1065, #1156 --- src/addrman.cpp | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/src/addrman.cpp b/src/addrman.cpp index c1a0df6a4..10d005aae 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -130,6 +130,7 @@ int CAddrMan::SelectTried(int nKBucket) // random shuffle the first few elements (using the entire list) // find the least recently tried among them int64 nOldest = -1; + int nOldestPos = -1; for (unsigned int i = 0; i < ADDRMAN_TRIED_ENTRIES_INSPECT_ON_EVICT && i < vTried.size(); i++) { int nPos = GetRandInt(vTried.size() - i) + i; @@ -137,11 +138,13 @@ int CAddrMan::SelectTried(int nKBucket) vTried[nPos] = vTried[i]; vTried[i] = nTemp; assert(nOldest == -1 || mapInfo.count(nTemp) == 1); - if (nOldest == -1 || mapInfo[nTemp].nLastSuccess < mapInfo[nOldest].nLastSuccess) + if (nOldest == -1 || mapInfo[nTemp].nLastSuccess < mapInfo[nOldest].nLastSuccess) { nOldest = nTemp; + nOldestPos = nPos; + } } - return nOldest; + return nOldestPos; } int CAddrMan::ShrinkNew(int nUBucket) From bd1aabe941bb7568e6e4e06356a6e98a8ba78a5d Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 5 May 2012 21:27:52 +0200 Subject: [PATCH 3/3] Bugfix: store source address in addrman --- src/addrman.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/addrman.h b/src/addrman.h index 7652df66a..3768614cf 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -62,7 +62,7 @@ public: nRandomPos = -1; } - CAddrInfo(const CAddress &addrIn, const CNetAddr &addrSource) : CAddress(addrIn) + CAddrInfo(const CAddress &addrIn, const CNetAddr &addrSource) : CAddress(addrIn), source(addrSource) { Init(); }