Skip to content

Commit

Permalink
refactor(peerfinder): use LogicError in PeerFinder::Logic (#4562)
Browse files Browse the repository at this point in the history
It might be possible for the server code to indirect through certain
`end()` iterators. While a debug build would catch this problem with
`assert()`s, a release build would crash. If there are problems in this
area in the future, it is best to get a definitive indication of the
nature of the error regardless of whether it's a debug or release build.
To accomplish this, these `assert`s are converted into `LogicError`s
that will produce a reasonable error message when they fire.
  • Loading branch information
scottschurr authored Oct 18, 2023
1 parent be6ac7e commit b69156a
Showing 1 changed file with 27 additions and 8 deletions.
35 changes: 27 additions & 8 deletions src/ripple/peerfinder/impl/Logic.h
Original file line number Diff line number Diff line change
Expand Up @@ -416,10 +416,9 @@ class Logic
// assert later when erasing the key.
slot->public_key(key);
{
auto const result = keys_.insert(key);
[[maybe_unused]] bool const inserted = keys_.insert(key).second;
// Public key must not already exist
assert(result.second);
(void)result.second;
assert(inserted);
}

// Change state and update counts
Expand All @@ -434,7 +433,11 @@ class Logic
if (slot->fixed() && !slot->inbound())
{
auto iter(fixed_.find(slot->remote_endpoint()));
assert(iter != fixed_.end());
if (iter == fixed_.end())
LogicError(
"PeerFinder::Logic::activate(): remote_endpoint "
"missing from fixed_");

iter->second.success(m_clock.now());
JLOG(m_journal.trace()) << beast::leftw(18) << "Logic fixed "
<< slot->remote_endpoint() << " success";
Expand Down Expand Up @@ -858,7 +861,11 @@ class Logic
{
auto const iter = slots_.find(slot->remote_endpoint());
// The slot must exist in the table
assert(iter != slots_.end());
if (iter == slots_.end())
LogicError(
"PeerFinder::Logic::remove(): remote_endpoint "
"missing from slots_");

// Remove from slot by IP table
slots_.erase(iter);
}
Expand All @@ -867,15 +874,23 @@ class Logic
{
auto const iter = keys_.find(*slot->public_key());
// Key must exist
assert(iter != keys_.end());
if (iter == keys_.end())
LogicError(
"PeerFinder::Logic::remove(): public_key missing "
"from keys_");

keys_.erase(iter);
}
// Remove from connected address table
{
auto const iter(
connectedAddresses_.find(slot->remote_endpoint().address()));
// Address must exist
assert(iter != connectedAddresses_.end());
if (iter == connectedAddresses_.end())
LogicError(
"PeerFinder::Logic::remove(): remote_endpont "
"address missing from connectedAddresses_");

connectedAddresses_.erase(iter);
}

Expand All @@ -894,7 +909,11 @@ class Logic
if (slot->fixed() && !slot->inbound() && slot->state() != Slot::active)
{
auto iter(fixed_.find(slot->remote_endpoint()));
assert(iter != fixed_.end());
if (iter == fixed_.end())
LogicError(
"PeerFinder::Logic::on_closed(): remote_endpont "
"missing from fixed_");

iter->second.failure(m_clock.now());
JLOG(m_journal.debug()) << beast::leftw(18) << "Logic fixed "
<< slot->remote_endpoint() << " failed";
Expand Down

0 comments on commit b69156a

Please sign in to comment.